-
Notifications
You must be signed in to change notification settings - Fork 1
Intermezzo
The review can be found right here and in the branch review right here
###Design: Its visible that the design is well thought out!
+The helper objects mailer, render and string_checker give the design a crisp structure
by keeping the logic in the models.
+The design has a rich OO domain model and each object has clear responsibilities.
+Invariants are well defined and cover the important aspects.
+Overall the code is well organized & reused, no code is duplicated.
Methods and Controllers were found, where they are expected to be.
-There is a violation of MVC layers because views and controllers
don't implement any business process.
###Coding style: All in all the coding style is consistent and clean. Method names emphasize the purpose. Classes are well organized. There is almost no duplicate code, but there are though some code rudiments in the comments, which should be deleted. --Encapsulations?
###Documentation: In general the Documentation is good and understandable.
All models have an informative class description where its needed
and the descriptions reveal the responsibilities.
The methods and attributes are well documented, but the method documentations
miss the pre and post conditions you use.
The user of the model has to know beforehand what the input of a method should look
like.
All controllers except
*organization
*registration
*account edit
miss documentation and comments!
I think its ok to drop the documentation in the redirect controllers, but for example
"item_actions" seriously needs code comments and controller descriptions.
The Comments and Descriptions match a consistent domain vocabulary.
###Test: The test coverage is high. The tests are easy to read, comprehensible. Testcases are distinct and represent common scenarios. Their names are chosen carefully and clearly state what is being tested.
###Conclusion: We liked to work with your code and the design of the website. For the most part the code was quite easy to use, except the undocumented controllers;).
Keep up the good work! Best regards Team 5.
- Violation of MVC layers
- Usage of helper objects between view and model
- Rich OO domain model
- Clear responsibilities
- Sound invariants
- Overall code organization & reuse, e.g. views
- Code is reused where possible
- Consistency
- Intention-revealing names
- Ruby idioms
- Do not repeat yourself
- Exception, testing null values
- Encapsulation
- Assertion, contracts, invariant checks
- Utility methods
- Exists?
- Understandable
- Intention-revealing
- 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
- Readability