FRAME_CONTENT_R0

[Astea]

wiki page: FRAME_CONTENT_R0 - effort: 1d

[deyan]

Adding category

[peko]

It is generally OK. The score is 3p because the task requirements section overlaps the overview section. It is understandable what should be done but it is not well structured. A little more information can be added. Ensure this is done in design section.

[tanya]

• Change the parent property of the FrameContent. It is Frame not FrameContent.
• "How to demo" should be use case, not unit test. What is more, do not changed the analysis after it has pass review. Because it is extra time, and it is possible to be wrong after that.

[pap]

• Design issues:
  • The FrameContent class is part of BASE_MODEL_FRAME_CONTENT task, so it is already existing
  • You should define how the ContentView of each FrameContent is to be created
  • The FrameView class may be responsible for creating the ContentView corresponding to its FrameContent, but as you are implementing the concept of ContentView it is your responsibility to add this functionality to the FrameView
  • You should define how non-defined content is going to be handled(displayed). Now non-defined content is represented as a Frame whose content property holds a null
  • Perhaps it's not a good idea not to have a ContentView at all as the user would like to see something.
  • You should define how content replacement is going to be handled.
  • Is it really necessary for the ContentView to know its parent FrameView? From what I see in the requirements of the task I don't see any use of that. You could discuss this with someone else.
• Implementation issues
  • The ContentViewFactory is not well designed. It seems stateless so its createContentView better be static.
  • This could make the contentViewFactory property of the FrameViewToDo class useless.
  • The createContentView method of the ContentViewFactory class has an useless final modifier to its argument
  • The compute method of the contentViewFactory property of the FrameViewToDo class should have an "assert getLastValue() == null;" statement. This is a general advice when using auto properties as one time set value properties.
  • It depends on the way you redesign content view creation but the compute method of the contentView property of the FrameViewToDo class may lead to some ambiguity. I mean that null value may mean different things. This could also be discussed.
  • The ContentViewTest should have methods that test content replacement and non-defined content. You actually have something like that but it's all in one test method.
  • In the ContentViewTest in the DummyContentView class it is a bad idea to use a value property the way you do with element(). The problem is that a new DefaultSceneElement will be created each time that you query the value of the property as long as you haven't set it to a non-null one.
  • I don't get the idea of the DummyFrameView class. The JavaDoc says about problems but it doesn't mention them.
• General issues
  • It is better if you link changesets in the result. It makes reviewing much easier.
  • Also you didn't show all the classes that you made changes to.
  • The testing section is about manual tests (the testing phase of each task).
  • Unit tests should be linked in the implementation section. Perhaps you could create a subsection if you think this makes the page look better.
  • Make sure you keep the wiki page of the task up-to-date with all the changes. High quality documentation is very important.

[pap]

• It is sufficient, but you should write more descriptive designs.
• Also I suppose that by template type you mean a generic. Please use correct terminology

[jani]

• It looks OK.
• Currently, UndefinedContentView is implemented as an empty image (ImageSceneElement). This is a subject to change.
• Please link all the changesets that are related to this task, even if there are minor corrections to some classes.

[deyan]

Batch update from file query-obsoleted.csv