albertionut11 / onlineshop_java

0 stars 0 forks source link

Code review 2 #2

Open vladandrei14 opened 1 year ago

vladandrei14 commented 1 year ago

Bug: Brand:46 -> incorrect assigment in getter DatabaseReaderService: A lot of reasorce leaks might happen due to improper closing of PreparedStatement (34 times). This could have been avoided if using try with resources DatabaseWriterService: Same as above. Also you used generic exceptions (eg RuntimeExceptions) instead of creating your own custom exceptions. Minor thing but a lot in the code: class fields should use camel case naming not "_" Main: unused imports MenuService: Cognitive complexity is high. This is a measure of how hard the control flow of a method(Start() in this situation) is to understand. This will also impact maintainability. The solution to this is to extract blocks of code in separate methods
MenuService: unused imports, unused class fields (dbService), class is not in a package Menu Service: methods should start with lower case characters Start() -> start() Minor thing: while loop has no ending

Poate am fost un pic dur cu notarea, insa cele mai "suparatoare" lucruri au fost resourse leak-urile(care erau multe), bucata de cognitive complexity si mici lucruri de coding standards. In acelasi timp apreciez complexitatea proiectului si faptul ca a indeplinit foarte bine cerintele. Am modificat nota din 8 in 8.50 (la review) ceea ce ar rotunji nota finala la 9.00.

albertionut11 commented 1 year ago

Multumesc mult!

On Wed, 7 Jun 2023 at 10:53, vladandrei14 @.***> wrote:

Bug: Brand:46 -> incorrect assigment in getter DatabaseReaderService: A lot of reasorce leaks might happen due to improper closing of PreparedStatement (34 times). This could have been avoided if using try with resources DatabaseWriterService: Same as above. Also you used generic exceptions (eg RuntimeExceptions) instead of creating your own custom exceptions. Minor thing but a lot in the code: class fields should use camel case naming not "_" Main: unused imports MenuService: Cognitive complexity is high. This is a measure of how hard the control flow of a method(Start() in this situation) is to understand. This will also impact maintainability. The solution to this is to extract blocks of code in separate methods MenuService: unused imports, unused class fields (dbService), class is not in a package Menu Service: methods should start with lower case characters Start() -> start() Minor thing: while loop has no ending

Poate am fost un pic dur cu notarea, insa cele mai "suparatoare" lucruri au fost resourse leak-urile(care erau multe), bucata de cognitive complexity si mici lucruri de coding standards. In acelasi timp apreciez complexitatea proiectului si faptul ca a indeplinit foarte bine cerintele. Am modificat nota din 8 in 8.50 (la review) ceea ce ar rotunji tota finala la 9.00.

— Reply to this email directly, view it on GitHub https://github.com/albertionut11/onlineshop_java/issues/2, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYHCD2QC36HJB455XQFLZSLXKAXQBANCNFSM6AAAAAAY5OZXHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>