What to do when code submitted for code review appears to be too complicated

code-reviews

The code is difficult to follow but it appears to be (mostly) working well, at least with superficial testing. There might be small bugs here and there but it's very hard to tell by reading the code if they are symptomatic of deeper issues or simple fixes. Verifying overall correctness manually via code review however is very difficult and time-consuming, if it is even possible at all.

What is the best course of action in this situation? Insist on a do-over? Partial do-over? Re-factoring first? Fix the bugs only and accept the technical debt? Do a risk assessment on those options and then decide? Something else?

Best Answer

If it cannot be reviewed, it cannot pass review.

You have to understand that code review isn't for finding bugs. That's what QA is for. Code review is to ensure that future maintenance of the code is possible. If you can't even follow the code now, how can you in six months when you're assigned to do feature enhancements and/or bug fixes? Finding bugs right now is just a side benefit.

If it's too complex, it's violating a ton of SOLID principles. Refactor, refactor, refactor. Break it up into properly named functions which do a lot less, simpler. You can clean it up and your test cases will make sure that it continues to work right. You do have test cases, right? If not, you should start adding them.

Related Topic