Code Review Comments General

Code Review Comments

This page aims to support common code review comments and feedback to open-mSupply code, and related projects.

As a code reviewer, we recommend you point to the explanations here when referring to a common code review feedback. If you find yourself making a similar comment regularly add it here so we can improve our knowledge base and support new developers learning without repeating yourself too often.

Prefer a simple solution over a complex one

Experienced Developers often argue for a simple solution rather than a complex and re-usable approach. Why? Complex code is hard to read, debug and understand, which means it can take time to review, test and is more likely to contain errors.

We often end up with overly complex or complicated code when...

• Trying to anticipate future requirements that may never come
• Trying to optimise code performance
• Lack of experience with the code base, or time pressure

If one of these factors is encouraging you to choose a complex solution, it might be worth discussing your solution with another developer to get another brain on the task.

Obviously sometimes we need complex code, and some problems are inherently complex, we need to identify the issue.

When using this comment, it should always come with a suggestion on how to simplify the code.

Example (Psuedo Code):

Too Complex

type stringTransformer {
     stringTransformerFunction: fn(s:string)->string
     functionArguments: any
}

fn 

Suggestion

fn uppercaseFirstChar(s: string):string {
  return //TODO
}

See: https://go.dev/talks/2015/simplicity-is-complicated.slide#11

Note: Simplicity can be subjective, so take care when using this comment. Remember to respect other developers ideas and code, even if someone has gone about a change differently to what you might have, it doesn't mean it's bad or wrong.

Nit's Don't need to be actioned