search

BME-MIT-IET / ivt-ha-2024-pied-piper-1

ivt-ha-2024-pied-piper-1 created by GitHub Classroom
0 stars 0 forks source link

Manual Code Review #2

Open JayWoodroffe opened 6 months ago

JayWoodroffe commented 6 months ago

Performing manual code review on some part of the application (GitHub, Gerrit...)

All manual code review comments are posted on the commit "New Project" since there were no pull requests after uploading this project to the repository.

JayWoodroffe commented 6 months ago

@punketeee I'm almost finished reviewing the Activity classes. Can you take a look at the Model classes and the FirebaseUtil class?

punketeee commented 6 months ago

@JayWoodroffe Final comments have been added to the commit. I'll start working on the md file for this task now.

JayWoodroffe commented 6 months ago

Thank you for finishing that. I will compile some of the more pressing repeating issues I noticed in the code and add it here and then we can summarise these findings in the md.

@JayWoodroffe Final comments have been added to the commit. I'll start working on the md file for this task now.

JayWoodroffe commented 6 months ago

There were three major problems that I came across during the Manual Review process.

  1. Inconsistent delegation/separation of concerns and duties between the classes. This mostly occurred due to mis/underuse of the FirebaseUtil class. Many other classes and activities were accessing the database storage directly and querying the data they needed within the class, instead of using the Util class. This can make troubleshooting and scalability of the program much more difficult.
  2. Inconsistent naming conventions. Many examples of inconsistent abbreviation between method, class and variable names. Can make it harder to find the right method you need and also reduces the readability of the code.
  3. The final issue that came up a few times was with some of the activities' classes. Multiple activities had all of the code from the class within the onCreate() method, resulting in spaghetti code that isn't very easy to read. I would suggest using more helper methods within these activities to reduce code duplication and make the code more understandable.