dorukkilitcioglu
commented
8 years ago Code Clarity:
Code is perfectly readable, there are no issues with clarity.
Bugs:
Session is opened but never closed in /TagDaoImpl.java at getTagsByContext(String) method. Note that this is a repository wide situation that some opened sessions are not closed. They will need to be checked and debugged. The reason some of them are done this way is due to the way Hibernate's lazy-loading works. Large scale debugging is necessary to deal with this problem.
Missing Features:
There are no features that were supposed to be done in this commit that are not done. There are numerous stuff that have been added later in the history of the search branch that either bugfix or complement what was done in this commit. Perhaps this would be a better question if a code review is done on a seperate branch through a pull request.
Coding Conventions:
- Variable Names:
- All of them are fine except for SearchController.java's getTagsByContext(String) method, wherein the variable might be better named to tagContext since it is the name used by the Tag model. Rest are all aligned with Java's naming guidelines.
- Indentation:
- All indentation are according to Java's indentation guidelines. IntelliJ corrects them anyway.
- Comments:
- Since most of the methods written here are just basic methods that are a couple of lines long, there is no need for many comments. There is only one place that from what I've seen needs a comment, which is SearchController.java's searchByTag(String) method, which actually does have comments in explanatory places, hence there is no need for extra comments.
Incomplete Code Documentation:
None of the methods and classes are properly documented. SearchController class has a basic template but it is not in a valid (Doxygen) format and is not very informative.
Coverage with unit tests:
There are no unit tests for any of the methods. Most of them are untestable since they are session related and their output will change as the database changes. What we can do, however, is add range checks (Ex: Integer.MAXVALUE and Integer.MINVALUE) to those methods that return an integer. They will still need the database to be functional to work, but at least there won't be any overflows.
Code Modification:
Code can be modified easily. For example, I can easily change the minimum amount of posts returned by SearchController's searchByTag(String) method and get more posts returned when searching by a tag.
Construct style:
Classes are all compliant with Hibernate's, Spring's, and Java's guidelines. Methods are small and modular.
Code Analysis Tools:
On running IntelliJ Code Analyzer on the methods associated with the commit, I have seen that there are no significant suggestions given by the analyzer.
Overall, this is one of the cleanest and issue free commits I've ever seen. Great job, @gokcantali , and don't forget to be awesome.
gokcantali
Please perform a code review on the commit 2e1fd92b7a022a75721c1813a6f5e955c8ec52f3 before this midnight.