Skip to content
fmatter edited this page Nov 13, 2012 · 18 revisions

We were asked to review the project of ese2012-team5 according to the guidlines provided here.

Main use cases

Code review

According the these guidelines:

Coding style: The item.split(at) method does not do what is described in the comment, the newly created item actually has the quantity at instead of rest and vice versa for the current item. For the current stage of the project this doesn't really make a difference though.

Tests: Most test functions have clear names, it is evident what they do/test, the use cases in store_simulation_test are transparent. Database's test coverage is below 50%. user_test sets up instances of database.rb for every test without using it (since it is not needed for creating/manipulating/deleting users). There are unfinished tests in database_test.

Design

  • 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
    

Coding style

  • Consistency
    
  • Intention-revealing names
    
  • Ruby idioms
    
  • Do not repeat yourself
    
  • Exception, testing null values
    
  • Encapsulation
    
  • Assertion, contracts, invariant checks
    
  • Utility methods
    

Documentation

  • Understandable
    
  • Intention-revealing
    
  • Describe responsibilities
    
  • Match a consistent domain vocabulary
    

Test

  • 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