changes.txt - Toubic/Workshop-2-Design GitHub Wiki

Changes - Feedback 1 - Songho Lee and Sarpreet Singh Buttar:

  • Added folders for "View" and "Model" in the project source code to make it more organized.

"It would be easier for reviewers if you had your model classes and view class in separate folders." [1]

  • Renamed "Console" to "ConsoleProgram" in the diagrams to make them represent the source code better.

"Console class, which we assume program.cs in your case" [1]

  • Replaced arrow between "ConsoleProgram" (former called "Console") and "MemberDatabase" in class diagram because I used the wrong arrow. Realization arrow changed to dependency arrow.

"Arrow from console to MemberDatabase presents wrong information. It should be replaced to direct dependency." [1]

  • Added forgotten dependencies involving "ConsoleProgram", "Member" and "BoatType" in the class diagram.

"Console class, which we assume program.cs in your case, has direct dependencies on Member" [1]

Changes - Feedback 2 - Beppe Karlsson:

  • Edited the program so it won't crash when given the wrong option value, member id or boat index.

"The program works as it should, however it does crash when you give it the wrong input. For example if you try to add a boat to a member id that is not registered. No error messages are shown, the program just exits. Which is fine since the reqs stated we do not need to show error messages. But the program should not crash." [2]

  • Removed multiplicity values at the source end because they are not needed in the class diagram.

"The relation between classes does not need to be annotated in the same way as when making a Domain Model class diagram. Meaning you do not need to use multiplicity in the same way. For example multiplicity at the source end is not necessary. [1, Chapter 16.4 Ways to Show UML Attributes: Attribute Text and]" [2]

  • Removed extra arrows describing actions in sequence diagrams because they are more confusing than helping.

"The sequence diagrams are a little confusing. Instead of drawing three arrows to create a member and show that we are passing information it can be put as one arrow with a call. [1, Chapter 15.1 Sequence and Communication Diagrams, Figure 15.3 showing the creation of a new entity in a sequence diagram]" [2]

  • Enhanced Model-View separation by moving initalization and saving functionality of the member database from "Program.cs" to "MemberDatabase.cs".

"The Program class seems to be implemented as the view and the very first thing that happens in the Program class is database operations which would do better in a dedicated Database model. [2, L04 Model View Controller]" [2]

  • Removed view functionality from the "Memberdatabase" because it doesn't belong in the model.

"The MemberDatabase performs View functionality when printing each member to the view. To maintain a Model/View separation the MemberDatabase class should return a list of members to display. The printing should be done in the view, not in the model." [2]

    • intputMemberID function to decrease repetitive code.

"The code quality is overall good. However there are a few cases of repeating code (Console.WriteLine("Enter member Id:")) which could be broken out into its own function and just be called repeatedly. That would also take care of typos that easily creep up on you when you retype the same stuff over and over." [2]

  • Renaming variables "theMember" to "member" and "theType" to "boatType" to be more simplified and understandable.

"There are also a few cases of theMember for example which could just be named member for a cleaner code. [3, Chapter 2: Meaningful Names]" [2]

  • Changed accessibility levels for member id and list of boats of a member and functions in "program.cs" to make it more protected.

"Encapsulation could have been used more generously. When I first looked at the class diagram I thought the visibility modifiers were wrong since most of them were set to public for things I felt should have been private to maintain good OOP practice. For example name and social security number on member. For example on the member class there are currently 4 public fields. They should all be made private with get and set properties (if needed). As they are set now they can easily be changed outside the class without any form of check that it is done correctly." [2]

  • Simplified the class diagram because it was too detailed.

"The Class Diagram show the relationship between the classes correctly. However the classes themselves may be a bit too detailed. Though personally I agree that it seems more useful to have more details as that would facilitate programming." [2]

"A class diagram for the entire application, focus on relationships of the classes and important details (do not add every single attribute or operation)" [3]

General

  • Separated "Program.cs" to two files "Program.cs" and "ConsoleProgram.cs" to separate intialization of startup objects from the view.

"The main method will just be responsible for creating objects that we need" [4]

References:

  1. Buttar S. S. & Lee S., Peer review comments Tobias Johansson Songho Lee and Sarpreet Singh Buttar, 2016-10-10, https://github.com/songhokun/1DV607-HT16/blob/master/Workshop2/Grade2/peer%20reviews%20from%20us/Tobias%20Johansson.pdf

  2. Karlsson B., prtj Beppe Karlsson, 2016-10-10, https://github.com/beppek/1DV607/wiki/prtj

  3. Olsson T., Workshop 2 – Design, 2014-09-21, https://coursepress.lnu.se/kurs/objektorienterad-analys-och-design-med-uml/workshops-2/workshop-2-design/

  4. Olsson T., OOA/D Lecture 3, Part 2, 2016-09-13, https://www.youtube.com/watch?time_continue=591&v=ymvjb7wb3so