-
Notifications
You must be signed in to change notification settings - Fork 0
Intermezzo
meilae edited this page Nov 14, 2012
·
17 revisions
Team4's project
- Violation of MVC layers:
- No violation of MVC
- Usage of helper objects between view and model
- Rich OO domain model and clear responsibilities:
- The responsibilities of each class aren't very clear, because there are only few model classes
- responsibility of data management spread out on many classes(item, user, holding)
- Sound invariants:
- No invariants implemented.
- Overall code organization & reuse, e.g. views:
- Classes are unclearly structured.
- Consistency:
- The requires aren't consistent in every class.
- see item.rb: 6 and user.rb: 1 ( the requires are inside or outside the class or module)
- Intention-revealing names:
- Some method names aren't logically chosen, so confusion is caused.
- see user.rb: 232 (login method seems to set the password?!)
- Ruby idioms:
- Not everywhere Ruby idioms are used because of unfamiliarity.
- see user.rb:154 (blocks would be nice here)
- Do not repeat yourself
- Exception, testing null values:
- Null values are tested.
- Encapsulation
- Assertion, contracts, invariant checks:
- No assertion, contracts and invariant are implemented.
- Utility methods:
- The methods are mostly small, so repetitions of codes are minimized.
- Understandable:
- Documentation of classes are very well understandable.
- Intention-revealing:
- Special cases aren't noted. There a lot of unnecessary documentation and some complicated methods aren't documented. Some parts of the documentation don't match with the type of the parameters (inconsistency)
- see item.rb: 175 (itemid stated as Integer but actually a String is passed)
- Describe responsibilities
- Match a consistent domain vocabulary
- Clear and distinct test cases
- Number/coverage of test cases
- Easy to understand the case that is tested
- Well crafted set of test data:
- A lot of the tests are unnecessary, because they test the same cases.
- Readability