Looking for a code review to be carried out. The review should asses the style, formatting, naming, test coverage, with particular focus on hours of coding required for search and transfer functions.
The review should also answer the following questions:
How does the new code fit with the overall architecture?
Does the code follow SOLID principles, Domain Driven Design?
What design patterns are used in the new code? Are these appropriate?
If the codebase has a mix of standards or design styles, does this new code follow the current practices? Is the code migrating in the correct direction, or does it follow the example of older code that is due to be phased out?
Is the code in the right place? For example, if the code is related to Orders, is it in the Order Service?
Could the new code have reused something in the existing code? Does the new code provide something we can reuse in the existing code? Does the new code introduce duplication? If so, should it be refactored to a more reusable pattern, or is this acceptable at this stage?
Is the code over-engineered? Does it build for reusability that isn’t required now? How does the team balance considerations of reusability with YAGNI?
Readability & Maintainability
Do the names (of fields, variables, parameters, methods and classes) actually reflect the thing they represent?
Can I understand what the code does by reading it?
Can I understand what the tests do?
Do the tests cover a good subset of cases? Do they cover happy paths and exceptional cases? Are there cases that haven’t been considered?
Are the exception error messages understandable?
Are confusing sections of code either documented, commented, or covered by understandable tests (according to our preference)?
Does the code actually do what it was supposed to do? If there are automated tests to ensure correctness of the code, do the tests really test the code meets the agreed requirements?
Does the code look like it contains subtle bugs, like using the wrong variable for a check, or accidentally using an and instead of an or?
The above must be assessed with reference to the following:
Are there potential security problems with the code?
Are there regulatory requirements that need to be met?
For areas that are not covered with automated performance tests, does the new code introduce avoidable performance issues, like unnecessary calls to a database or remote service?
Does the author need to create public documentation, or change existing help files?
Have user-facing messages been checked for correctness?
Are there obvious errors that will stop this working in production? Is the code going to accidentally point at the test database, or is there a hardcoded stub that should be swapped out for a real service?