Ticket #276 (closed planned_task: obsolete)

Opened 14 years ago

Last modified 13 years ago

BASE_BOOK_COMMONS_R0

Reported by: Astea Owned by: boyan
Priority: 3 Milestone: M03_PRE3
Component: BASE_BOOK_MODEL Version: 2.0
Keywords: Cc:
Category: BASE Effort: 2
Importance: 35 Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: orliin Design_owners: boyan
Imp._owners: boyan Test_owners:
Analysis_reviewers: peko Changelog:
Design_reviewers: peko, peko Imp._reviewers: pap, jani
Test_reviewers: Analysis_score: 3.5
Design_score: 3.5 Imp._score: 3
Test_score: 0

Description

wiki page: BASE_BOOK_COMMONS_R0 - effort: 2d

Change History

comment:1 Changed 14 years ago by deyan

  • Category set to BASE
  • Design_score set to 0
  • Imp._score set to 0
  • Test_score set to 0
  • Analysis_score set to 0

Adding category

comment:2 Changed 14 years ago by peko

  • Importance changed from 0 to 35

comment:3 Changed 14 years ago by orliin

  • Owner changed from Astea to orliin
  • Status changed from new to s1a_analysis_started
  • Analysis_owners set to orliin

comment:4 Changed 14 years ago by orliin

  • Status changed from s1a_analysis_started to s1b_analysis_finished

finished in 1h

comment:5 Changed 14 years ago by peko

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_reviewers set to peko
  • Analysis_score changed from 0 to 3.5

comment:6 Changed 14 years ago by peko

The analysis is quite good. Maybe the task requirements section contains too much things that can be done in 2 days but it is not unachievable.

comment:7 Changed 14 years ago by boyan

  • Design_owners set to boyan
  • Owner changed from orliin to boyan
  • Status changed from s1c_analysis_ok to s2a_design_started

taken for design

comment:8 Changed 14 years ago by boyan

  • Status changed from s2a_design_started to s2b_design_finished

done in 5h

comment:9 Changed 14 years ago by peko

  • Status changed from s2b_design_finished to s1c_analysis_ok
  • Design_score changed from 0 to 2
  • Design_reviewers set to peko

Defining things about book commons is ok. But things that can be done with books are not all connected with the book model. The model is separated from the things that can be done with the model. In other words the model does not contain the View and the Logic since these are helper components that change the model and display it. In the design you have to state all the things that the model contains - frame connections, pages etc. And then should describe the hierarchy of classes that contain Books, Pages, Backgorunds, Connections etc. In implementation phase these should be implemented - or at last base classes, since this is huge to be done for the effort stated.

comment:10 Changed 14 years ago by boyan

  • Status changed from s1c_analysis_ok to s2a_design_started

taken for improvement

comment:11 Changed 14 years ago by boyan

  • Status changed from s2a_design_started to s2b_design_finished

redesigned in 2h

comment:12 Changed 14 years ago by peko

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 2 to 3.5
  • Design_reviewers changed from peko to peko, peko

Design is pretty good.

comment:13 Changed 14 years ago by boyan

  • Status changed from s2c_design_ok to s3a_implementation_started
  • Imp._owners set to boyan

taken for implementation

comment:14 Changed 14 years ago by boyan

  • Status changed from s3a_implementation_started to s3b_implementation_finished

Done in approximately 2.5d (total).

comment:15 Changed 14 years ago by pap

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 0 to 1
  • Imp._reviewers set to pap

Reviewed in 90m.
Things that are not OK:

  • There are no connections between the classes in the uml diagram.
  • Warnings in unit tests
  • assert* methods usage is not good. Prefer assertEquals/assertSame to assertTrue( foo == bar )
  • The test for current page is not good. You should test that if the page that is curent is removed from the book there is no urrent book and after readding the same page it becomes current
  • If the book is not supposed to be inherited(this means having different book types) then the class Book should be declared final. The same applies to Page.
  • The class (Book, Page) layout is not accoding to this convention http://java.sun.com/docs/codeconv/html/CodeConventions.doc2.html#1852 , which happens to be in the useful links page in thedev doc.
  • I think that the method createDefaultBook(String title, ImmSize pageSize) is not a good idea. It could either be a private helper method or it shouldn't exist. But I don't think that there can be many kinds of default books.
  • You should describe better the idea of lastCurrent
  • The version property of the book is not needed. there will be a version provided when we make the book a resource.
  • The delete method of the book class doesn't seem very sensible to me.
  • The frames method has a check that's not necessary after the @Own annotation has been impelmented - the one whether the frame is already added
  • There are unnecessary finals in the Page class
  • The getMaxPageBorderThickness() of the Page class is more like a Book method if it is necessary at all
  • There is at least a prototype for that clones ProObjects so it can be used for copying books.
  • The shadow() method of the Page class and the delete methods of the Page and Book classes should be reviewed and removed if they are no needed
  • PageIntegrationTest and BookIntegrationTest should be refactored. Remove the "@SuppressWarnings("all")" directive, write JavaDoc, refactor the test methods

comment:16 Changed 14 years ago by boyan

  • Status changed from s2c_design_ok to s3a_implementation_started

taken for improvement

comment:17 Changed 14 years ago by boyan

  • Status changed from s3a_implementation_started to s3b_implementation_finished

done in 1 eff.

comment:18 Changed 14 years ago by jani

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 1 to 3
  • Imp._reviewers changed from pap to pap, jani

The remarks from the previous implementation review are taken into account. I see no new problems and the unit tests pass. 50m.

comment:19 Changed 13 years ago by deyan

  • Status changed from s3c_implementation_ok to closed
  • Resolution set to obsolete

Batch update from file query-obsoleted.csv

Note: See TracTickets for help on using tickets.