I help companies achieve software excellence through clean architecture, rigorous code reviews, human-centered leadership and writing clean & maintainable code.
Follow SOLID principles.
17 years experience with Java.
15 years experience with SWT and Eclipse RCP.
5 years experience with Spring Boot, Microservices, REST API, Postman.
Strong skills about debugging in the code.
For more details about my experience please check the homepage.
Click to expand
-------------------------------------------------------------------------------------------------------------------------
- It's good that you didn't put any secret in the configuration files of the app and you decided to pass it through environment variables
- On top of my review you can use chatGPT Codex app to connect chatGPT to your github repo to review code, fix problems, change things. For all of the changes it will create pull requests so you can see it before you merge.
- It's very good that you put all the entities in the same package, all dto's in the same package and all services in the same packages.
- Setup the readme.md like you did for the frontend. I like how you did it on frontend repo.
- Consider creating a constant for system user in a utility class (for instance CommonUtils - see point 10 from this doc) and use that constant everywhere you need.
-------------------------------------------------------------------------------------------------------------------------
- Use the same version of spring boot for all spring dependencies. You used 3.1.1 only for parent and not for others. That is one of the reasons why the app doesn't start. And put the version in the properties section so will be easy to modify in the future (like you did for mapstruct.version).
- Remove the dependency to tomcat. You don't need that specific version.
-------------------------------------------------------------------------------------------------------------------------
- It's good that you decided to use liquibase to make changes in the database structure. About naming of the scripts I have a suggestion. When you work in a team and each of the members can write everyday liquibase scripts then considering only the date in the filenames is not enough.Also collisions could happen in the merge process. One suggestion is to use an index instead of timestamp. For instance 000001-add-food-for-user-12.xml, 000002-add_email_to_user.xml, etc
And to have a page somewhere on github or confluence where devs can "book" a new index which they want to use. Another suggestion is to keep the date but also add the time 20240702-140142-add-food-for-user-12.xml, 20240701-071236-add_email_to_user.xml. With the second method you might have problems when your colleagues lives in different countries, different timezones.
- You didn't create the rollback section in the liquibase. Because of that then you won't be able to rollback scripts in production with liquibase rollback command and then the only choice would be to fix manually in prod problems related to structure which is a bad idea. I know it's very annoying and time consuming to create a rollback section but it's for safety and all the customers appreciate that. Imagine a scenario when you deploy a new version of your app to prod, something is wrong with the app and the customer requests from you to un-deploy and to deploy the previous version. But your latest version already made changes to the db structure so the previous version of the app won't work until you modify the structure manually. In this situation for instance liquibase rollback command would be useful. So you rollback maybe 3-5 liquibase changesets and then you deploy the previous version of the app.
- You didn't create any sql index in the liquibase changesets when you define new tables. Those indexes are really helpful to speed-up searching in the table. But do not create indexes for all the columns or column combinations. Firstly create indexes for columns where you estimate you will search from the code. And consider also searching on two columns from the same table. If that happens in the code then you have to create an sql index for both of the columns. Also please pay attention to the index names. It would be good to be like this: <tablename>_<field1>_<field2>...
- It's very good that you defined foreign keys
-------------------------------------------------------------------------------------------------------------------------
- you don't need to use @Query for the methods if methods name and parameters are according to schema definition
- please check all the methods from ExercisesRepository
- please check all the methods from SetTrackingRepository
- please check all the methods from UserWeightRepository
- please check findByUserIdAndDate method from LoggerRepository
- please check findByUserId from WorkoutsRepository
-------------------------------------------------------------------------------------------------------------------------
- I observed you created constructors with parameters for all the services but there is a big inconvenience about this. For complex apps one service can have references to dozens of repositories or other services or other components. What will you do then? Will you add a constructor with 30 parameters? The solution is to use @Autowired for the fields from service class.
- Also for controllers I observed the same thing.
-------------------------------------------------------------------------------------------------------------------------
- checking if system user exists is non-sense because my expectation is that system user is added by liquibase script so it's guaranteed that it's in the database. Generally speaking you should not check in the code if something exists if that was added/set by liquibase script. And if someone modifies MANUALLY the database it's his problem and should not be allowed.
- the default exercises should be added by liquibase script and not from code.
-------------------------------------------------------------------------------------------------------------------------
- I like the way you named the REST API's with dash between words and you used the proper http methods (GET, POST, DELETE, PUT)
- Consider adding the parameter of the ResponseEntity so it's easy to understand the code. Also if you do that later if you implement swagger annotations for documentation then it will be easy to see the type of response. Example:
@PostMapping("add-exercise")
@PreAuthorize("hasAuthority(T(com.gym_website.util.enums.EAppRoles).MEMBER)")
public ResponseEntity<ResponseDto> addExercise (@RequestBody ExercisesDto exercisesDto){
return ResponseEntity.ok(exercisesService.addExercise(exercisesDto));
}
…
@GetMapping("/get-set/{id}")
@PreAuthorize("hasAuthority(T(com.gym_website.util.enums.EAppRoles).MEMBER)")
public ResponseEntity<SetTrackingDto> getSetTracking(@PathVariable("id") Long id) {
return ResponseEntity.ok(setTrackingService.getSetTracking(id));
}
- In the SetTrackingController class remove the / before the endpoints name because it is not needed. For instance here @PostMapping("/add-set")
- In ExercisesController, add-exercise endpoint, the request body contains an ID but i guess the frontend when calling that endpoint will not pass any ID because it's the first time when it is created. So the classes which are used for request bodies should not contain fields which are not set ever by the frontend. The solution for that endpoint is to create a new class called RequestAddExercise which contains only the fields which are set by the frontend. Please do similar checks for all the endpoints of all controllers. For the response classes it's similar. Instead of using a generic DTO which also contains fields which are never set the endpoint consider creating dedicated classes for endpoints with Response prefix. I know is not convenient but in the long term the code is easy to maintain because you can easily add/remove a new field from a body of an endpoint without affecting other endpoints.
-------------------------------------------------------------------------------------------------------------------------
- consider renaming LoggerController, LoggerService, LoggerRepository, LoggerEntity because it's confusing with logging mechanisms. You do not store in the base the logs of your app but you store the history of user changes. Suggestions for LoggerEntity: UserTrackerEntity or UserHistoryEntity.
- consider renaming SetTrackingController because of previous suggestions
-------------------------------------------------------------------------------------------------------------------------
- in this class there is only one method and that is used only in one controller. From a design perspective this is not good. Instead I recommend you to throw an exception in the controllers (your own exception which extends RuntimeException) and then you create a handler for all exceptions of the API's and use @ControlleAdvice for that. Please do a little bit of research about this. The idea is in that class with @ControlleAdvice you can define a method for each type of exception (triggered in controllers) which you want to catch and you have the control over the response of the API so you can set a specific error code and body.
-------------------------------------------------------------------------------------------------------------------------
- In Spring Boot as many services you have as heavy/lazy the app becomes. For 100 services it might still be ok but why to implement a service where it's suitable for a utility class? Also in LoginUtilService class you have static methods which violates the concept of a service in spring boot. So my suggestion is to replace it with this
public final class CommonUtils {
private CommonUtils() {
// Prevent instantiation
}
public static EAppRoles getRole(UserDto userDto) {
if (EAppRoles.MEMBER.label.matches(userDto.getRole())) {
return EAppRoles.MEMBER;
}
return EAppRoles.MEMBER;
}
public static Cookie createJwtCookie(String jwt) {
Cookie cookie = new Cookie("auth-cookie", jwt);
cookie.setPath("/");
cookie.setDomain("localhost");
cookie.setHttpOnly(false);
return cookie;
}
public static LocalDateTime getTime (){
return LocalDateTime.now();
}
Also remove TimeUtilService class.
-------------------------------------------------------------------------------------------------------------------------
- In ExercisesController, delete-exercise-{id}, any user can delete any exercise from others based on id which is not good from a security perspective. The solution is to check in the code if that id belongs to the user who passed the request. Please do similar checks also in other controllers.
- Generally speaking in controllers it's good to test the parameters before using them in the service and repository levels. For instance if you expect a maximum 100 characters for a string parameter then consider checking in the code for 200 characters, so bigger than expected. But without any limitation the hacker can call the API with a generated string of 1GB which can make the app a little bit slower and also can cause a log forging attack if that parameter is logged. Or even worse, the hacker could inject huge values in the database.
-------------------------------------------------------------------------------------------------------------------------
- Such things can be optimized and perform at the end just one query for deletion. And you put that in a try-catch-exception. If exception is triggered then you catch it and you trigger your own runtime exception with specific message
If (workoutsRepository.findById(id).isPresent()) {
workoutsRepository.deleteById(id);
Leaving the code as it is it will result in performing two queries on the database one for find, one for delete. Such optimizations you can do in many other places too. Please check.
- I saw many places where actually you don't need to return anything meaningful on the response, like this
@PutMapping("edit-exercise")
@PreAuthorize("hasAuthority(T(com.gym_website.util.enums.EAppRoles).MEMBER)")
public ResponseEntity editExercise(@RequestBody ExercisesDto exercisesDto){
return ResponseEntity.ok(exercisesService.editExercise(exercisesDto));
}
In this case the response object is just like this
ResponseDto responseDto = new ResponseDto();
responseDto.setSuccessMessage("Exercise modified");
That's not required. Instead you can make the method from controller to return void so code is more simplified. Like this
@PutMapping("edit-exercise")
@PreAuthorize("hasAuthority(T(com.gym_website.util.enums.EAppRoles).MEMBER)")
public void editExercise(@RequestBody ExercisesDto exercisesDto){
exercisesService.editExercise(exercisesDto);
}
Feel free to ask me for the contact details of the following people so you can personally verify with them the references they provided for me.
Tibor Cseke - Project Manager at NTT DATA - 2024
Our collaboration with Cristian Tone on the project from June 2021 to December 2024 has far exceeded our expectations, and we sincerely hope to work together again on future projects.
His professionalism, exceptional team coordination, and effective communication with the end client have made this partnership exemplary.
The end client was highly impressed by the speed at which NTT DATA delivered all requested functionalities, a success largely attributed to Mr. Cristian. Additionally, the developers who worked with Cristian Tone at NTT DATA were extremely satisfied with the processes he implemented within the team and the way he led the team.
Thank you Cristian Tone for an outstanding partnership!
Joachim Ruckaberle - Business Product Owner at Mercedes-Benz AG - 2024
I am writing to recommend Cristian Tone. He worked with us at Mercedes-Benz AG as a Lead Developer and Project Architect and worked with me in my position as Product Owner.
As an employee, Cristian Tone was always passionated and has a solid technical expertise. During his time in my team, he managed to deliver the requested features in time and quality and handle problems in production very fast with provided hotfixes.
I’ve always put a premium on independence among my team members and Cristian Tone never failed to deliver. An example was when problems with third party systems arise he takes over the communication with the third party team to solve the problem.
Cristian Tone is a delight to work with and I wouldn’t hesitate to hire him again.
Irfan Ahamed - Lead Developer at Mercedes-Benz AG - 2024
I had the pleasure of working with Cristian for almost a year, and I was consistently impressed by his professionalism, flexibility, and strong leadership skills — all essential qualities for an outstanding Tech Lead.
Cristian combines these skills with exceptional expertise in Java and Rich Client Applications, making him an invaluable asset to any project.
His ability to lead, adapt, and deliver results is truly amazing. Any team would be lucky to have Cristian, and I truly miss having him on mine.
Eduard Suranyi - Java Developer at Konecranes/TBA - 2018
In the first 2 years he was the one who taught me to write graphical interfaces and programming in general, he was the one who laid the foundations in my training as a developer.
He offered me support and was always available when I needed it. In discussions with him he was sociable. The predominant qualities of him were those related to the creation of graphical interfaces, 2D and 3D graphics, advanced mathematical calculation.
From the point of view of the responsibilities passed on after the departure, the notions of programming learned from him and the tasks on the projects he worked on also made it easier for me to take on the responsibilities further, moreover, my team was fully confident that I knew how to implement anything in the applications he worked on.
Bernhard Goldemund - Administrator at QTronic/Synopsys - 2019
We are sending this letter of recommendation in order to introduce Mr. Nelu-Cristian Tone, an employee of our company during the period 23/04/2018 – 19/11/2019, holding the position of programmer. His main responsibilities consisted of:
Developing software according to specifications.
Designing software components using design patterns.
Analyzing alternative technologies for implementing software components.
During the above-mentioned period, he performed according to expectations for the position held.
Had an appropriate attitude towards superiors and colleagues.
Had no disciplinary sanctions in the last 12 months.
Did not cause any material or reputational damage to our organization.
To our knowledge, is not subject to any ongoing criminal or administrative proceedings.
Ruxandra Bănici - Project Manager at Zenitech - 2020
Cristian Țone was a Zenitech employee between January and May 2020, holding the position of Senior Developer. During this period, Cristian worked directly with a major client of our company, being part of the client's team. During the hiring process, Cristian also went through interviews with the client, who selected him from several candidates because they found a good match in terms of seniority and openness to innovation. Cristian had a time of adjustment in which he managed to get acquainted with a new technology for himself and managed to work in a short time on the same tasks as his colleagues in the client's team. During the project Cristian was involved in any type of task needed to bring improvements to the project. In addition to development tasks, he also took on tasks to improve the way he works, for example: he understood and documented automatic testing and release processes in order to bring these capabilities within the team and not to depend on other departments. Already working fully in the client's team, the transition to working from home due to the pandemic was made without any impact on productivity and collaboration. During the short time we worked together, I noticed in Cristian both his passion for technology and his desire to collaborate as well as possible with his teammates.
Patrick Morăraș - Production Manager at Funlabs/Activision - 2007
Cristian worked at our company for about two years, completing his assigned tasks on time and to the required quality standard. If life hadn’t forced him to relocate, I’m sure he would still be working with us today. His reserved and calm nature did not stop him from speaking up when encountering problems. When faced with difficult tasks, he didn’t give up and, with help from colleagues and/or the project lead, found solutions and completed the tasks within the required parameters.
Andrei Streche - Lead Project Programmer at Funlabs/Activision - 2007
I was a colleague of Cristian for a year and a half. During this time, I appreciated his seriousness, punctuality, and dedication. With an open attitude towards new things, Cristian made significant progress as a programmer during his time at FUNlabs, becoming a valued team member. He worked on various modules, from mission scripting to UI and visual effects. He maintained very good working relationships both with colleagues from the programming department and with those from other departments he collaborated with (2D graphics, FX, etc.).
Emil Anghel - Lead Game Designer at Funlabs/Activision - 2007
Cristian Tone made a significant contribution to the projects he worked on, fulfilling his tasks with professionalism and enthusiasm. He integrated very well into the programming team, becoming a core member shortly after starting work at Fun Labs. I greatly appreciated the fact that he approached his tasks with the utmost seriousness and dedication and maintained the same attitude throughout a long and important project.
I’m glad I had the chance to work with him and I wish him all the best.
My last collaboration was with NTT DATA on the same project for about 3.5 years
Efficiency: Achieving expedited software development lifecycles.
High-Quality Code: Developing clean, structured, and maintainable code in adherence to SOLID principles.
Process Optimization: Identifying and implementing strategic enhancements to refine team workflows.
Transparency: Maintaining comprehensive documentation of all processes and agreements to ensure clarity and alignment.
Highly organized and able to prioritize tasks effectively under pressure.
Take ownership and responsibility.
Effective collaboration with cross-functional teams.
Proactive with a solution-oriented mindset.
Human-centered leadership abilities.
Open to learn new technologies.
I own a high-performance DELL laptop powered by an Intel i9 processor with 24 cores and 32 GB of RAM and Microsoft Windows Pro 11.
I own a Blue Yeti external professional microphone, which delivers exceptionally clear audio quality.
In addition, I have a dedicated workspace where I can work without interruptions.
I own an external battery that can keep both the laptop and external monitor running for at least 6 hours, which is very handy during a power outage.
I have a stable and fast internet wired connection (up to 1 GB/s) from Orange https://www.speedtest.net/result/18201493175
I run my own company, pay taxes to the government, and can sign contracts for software development with companies anywhere in the world.
My current company:
https://www.risco.ro/en/verifica-firma/tone-nelu-cristian-persoana-fizica-autorizata-cui-52149812
Tax Identification Number (TIN): 52149812
My previous company:
https://www.risco.ro/en/verifica-firma/herbs-house-cui-47356056
Tax Identification Number (TIN): RO47356056
VAT number: RO47356056