search

iasonasma / seipassignment

The second assignment of SEIP
0 stars 0 forks source link

Metric Analyzer - PR #13

Closed iasonasma closed 4 years ago

iasonasma commented 4 years ago

Hello!

I've done the changes based on the 3rd assignment for principles of design. I'm deeply sorry for my english mistake. Inform me please if I have forgotten something on details on README or something isn't cleared. Also I've added the bonus for travis with a correct csv file and also a csv file with a mistake.Results are one will have a error and the other will be correct.

Thank you!

iasonasma commented 4 years ago
  • Missing test cases in Facade and in Factory
  • Each hierachy (analyzers, exporters, readers) is a Strategy pattern. You don't mention this in the documentation
  • Each Strategy should have its own Factory in order to be an independent subsystem. Now all subsystems are bound to each other due to your single Factory
  • Classes should be grouped into logical java packages (analyzers in one package, exporters in another, etc)
  • Missed the Bridge pattern between the Analyzers and the FileReaders. (See the proposed solution (when it becomes available) for the benefits of using this pattern)

Well done, you can now merge with master.

Should I fix those for a greater grade?

AntonisGkortzis commented 4 years ago
  • Missing test cases in Facade and in Factory
  • Each hierachy (analyzers, exporters, readers) is a Strategy pattern. You don't mention this in the documentation
  • Each Strategy should have its own Factory in order to be an independent subsystem. Now all subsystems are bound to each other due to your single Factory
  • Classes should be grouped into logical java packages (analyzers in one package, exporters in another, etc)
  • Missed the Bridge pattern between the Analyzers and the FileReaders. (See the proposed solution (when it becomes available) for the benefits of using this pattern)

Well done, you can now merge with master.

Should I fix those for a greater grade?

Unfortunately, that is not an option for this assignment.