Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code review #17

Merged
merged 32 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
01511c4
POM.xml organizing
sayfjawad Dec 20, 2024
175e21a
first step refactoring
sayfjawad Dec 20, 2024
8a0dc0d
code styling
sayfjawad Dec 20, 2024
5e5bdba
More refactoring, better constructors
sayfjawad Dec 21, 2024
860d744
More refactoring, prepare to split responsibilities
sayfjawad Dec 21, 2024
4e8002b
More refactoring, prepare to split responsibilities
sayfjawad Dec 21, 2024
0cca93b
More refactoring, prepare to split responsibilities
sayfjawad Dec 23, 2024
e37080d
More refactoring to responsibility
sayfjawad Dec 23, 2024
9ed0b30
More refactoring to remove unnecessary code
sayfjawad Dec 23, 2024
119bdb7
More refactoring to remove unnecessary code
sayfjawad Dec 23, 2024
46254c7
More refactoring to become more SOLID
sayfjawad Dec 23, 2024
809e10f
Making sense of the madness
sayfjawad Dec 23, 2024
b22f4ef
More refactoring, split responsibilities
sayfjawad Dec 23, 2024
76b45a0
More refactoring, split responsibilities
sayfjawad Dec 23, 2024
a41e7da
More refactoring, split responsibilities
sayfjawad Dec 23, 2024
a4de7f4
More refactoring for better naming
sayfjawad Dec 23, 2024
8f08c02
More refactoring to improve singe responsibilities
sayfjawad Dec 23, 2024
cabbffd
Added mutation testing coverage with PiTest
sayfjawad Dec 23, 2024
6dddccf
Making controllers and services more uniform
sayfjawad Dec 24, 2024
42ce262
refined error handling
sayfjawad Dec 24, 2024
0c85a8b
More coverage and unit testing
sayfjawad Dec 24, 2024
b21f43d
More coverage and unit testing
sayfjawad Dec 24, 2024
cc517dc
More cleanup
sayfjawad Dec 24, 2024
4240230
More unit testing
sayfjawad Dec 25, 2024
e9e05e7
95% mutation coverage and some good old SOLID refactoring
sayfjawad Dec 26, 2024
4d3c8d0
95% mutation coverage and some good old SOLID refactoring
sayfjawad Dec 26, 2024
2b97626
Uniformity in unit testing and integration tests
sayfjawad Dec 26, 2024
1c73c8c
Uniformity
sayfjawad Jan 3, 2025
bec6e3a
Een aantal punten die opgepakt zijn ter verbetering
sayfjawad Jan 3, 2025
7be8eaa
Een aantal punten die opgepakt zijn ter verbetering
sayfjawad Jan 3, 2025
192def6
Merge pull request #3 from ICTU/development
sayfjawad Jan 4, 2025
abbabc9
Merge branch 'development' into code-review
sayfjawad Jan 4, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions Verbeteringen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
Plus punten:
Java 21
Heel lief simpel klein applicatie en nog prima in orde te maken omdat het nog niet te laat is!
- weinig complexiteit

-------------------------------
-------------------------------
Beoordeeld op basis van de ISO-25010 maintainability kwaliteiten:
* Modulariteit
* Herbruikbaarheid
* Analyseerbaarheid
* Aanpasbaarheid
* Testbaarheid

Het volgende:

* Niet Modulair:
- een enkele module

* Moeilijk analyseerbaar
- teveel verschillende verantwoordelijkheden verzameld in een plek
- code duplicatie
- weinig tot geen documentatie (ook m.b.t. OpenAPI contract)
- geen duidelijke benamingen van classes op basis van verantwoordelijkheid

* Moeilijk herbruikbaar:
- een enkele module met alle verantwoordelijkheden
- geen documentatie
- geen garantie op functionaliteit door ontbreken test coverage

* Moeilijk aanpasbaar:
- elke aanpassing heeft grote gevolgen door grote stukken code
- weinig segmentatie

* Moeilijk testbaar:
- erg weinig segementatie in verantwoordelijkheden
- teveel op een plek
-------------------------------
-------------------------------

Te verbeteren
README.md mag veel meer informatie bevatten
* wat is het
* waar dient het voor
* hoe werkt het opzetten
* wie is ervoor verantwoordelijk
* etc

Maven project
- versienummers kunnen beter als properties verntraal worden beheerd
- een mutatie test kan iets meer inzicht geven in hoe goed het getest wordt

Documentatie:
* veel complexe cryptography vergt wel het een en ander aan uitleg

Checkstyle & psot-bugs:
- veel warnings die suppressed worden kunnen net zo goed eruit of gefixed worden

Testing
- 3 testen voor een hele applicatie is geen garantie dat de applicatie doet wat het moet doen
- test coverage is erg laag
- MUTATIE coverage is nog lager
- geen duidelijke beschrijving van tests en wat ze moeten doen
- enkel integratie testing en geen unit tests
- geen mocks
- niet makkelijk te testen vanwege
- onwenselijke instantiaties van on-mockbare objecten
- onduidelijkheid en verwevenheid in verantwoordelijkheden (teveel in een plek)
- integratie testen betekent simpelweg dat je bij elke aanpassing alle testen moet herzien

Controllers:
- weinig validatie op input!!!
- doen teveel!
- @SneakyThrows probleem
- benaming
- moeilijk testbaar
- moeilijk leesbaar
- niet gedocumenteerd

Services:
- doen teveel! houd het bij controlleren van input
- @SneakyThrows probleem
- benaming
- moeilijk testbaar
- moeilijk leesbaar
- niet gedocumenteerd
- duplicate code

Utils:
- combineren van Byte[] wordt om meerdere plekken gedaan maar niet consistent gebruikmakend van deze util

Configuratie:
- @Component vervangen met @Configuratioe
- zorg dat hier de validatie gedaan wordt i.p.v. constructors van classes die hier gebruik van maken
- test de configuratie

Models:
- te veel suppress warnings
- geen eigen package
- hard coded values
- maak het netter met LOMBOK

OpenAPI:
- documentatie ontbreekt
- maak het netter met LOMBOK

application-properties:
- waarom is PRD logging altijd op DEBUG?





Loading
Loading