6 Tips to Succeed in Code Review as a New-hire (w/o being bashed)

As a new-hire, it isn't always easy to adjust to the norm of the team.

Socially, people are usually friendly at the workplace, so you control the dynamics about how much you like to blend in. However, when it comes to code review, your friendly neighbor may bash on you repeatedly.

Why? Code convention matters and code quality is the key to keeping everyone's job.

I have six tips to help you catch up to the new team without being bashed.

1. Honor the context and lower your pride

If you are new to software engineering, then jump to the next section. Don't even think about pride. 😉

You may have years of working experience, but please, please be humble. None of your experience matters if you don't know the context.

A solid example of an argument from a new-hire. The claim was:

There must be a service-specific interface when integrating/calling a dependency service.
Reason: the external dependency service can change, but in our code, callers of the interface can stay w/o changes.

Code becomes like this:

public MetadataService implements MetadataServiceInterface{}
public PublishingService implements PublishingServiceInterface{}
public AService implements AServiceInterface{}
public BService implements BServiceInterface{}
... //10 more services like this

By looking at the statement, it sounds entirely correct. Yes, loose coupling from the dependency integration is a good engineering practice.

However, the good intention brings almost no value in our team's context: there is no need for decoupling.

• Unfortunately, the dependency services are not simple CRUD APIs. We cannot replace them.
• By design, the way (i.e., high-level workflow and business logic) we utilize these APIs in our code has already strongly coupled with the specific services.
• There is only one implementation of each service integration in the long term.
• If we do change a dependency, the business logic will be affected inevitably.
• Good intention practices like this will fail to achieve the very close launch dates, especially requiring changes to everything in our code base. The change will cause days of interruptive merging on multiple engineers' branches and unit test changes.

I am not saying these rejection comments are necessarily correct or conduct best engineering practice. The reality is, a team needs to produce, and we cannot delay the launch. One of the best qualities of software engineering is to take calculated risks, so you have to learn the context.

2. Read your team's code reviews

Afraid of making mistakes? Just learn from others' problems. You can very quickly figure out the common nit-picking issues, the concerning code structure, and the error handling practices.

You can also observe good practices as well. Find out who are the best engineers on the team and look at their code.

I'm not saying that you should follow what others do without critical thinking, but at least understand the dynamics. Don't make the same mistakes that others have made.

3. Leverage code convention wiki

If your new team already has a code convention wiki, that's amazing. You'll learn fast about what to do and not.

If your team does not have a code convention wiki, it's time to build one : )

I like to take notes of things that I learned and put them into a wiki. Remember all the mistakes and adjustments you read in others' code reviews? Document these in a wiki so everyone can learn.

Documenting code convention does not give you authority, but it groups you and the others under the same umbrella. You are contributing in a non-intrusive way, that always brings allies.

4. Start with a small code change

Regardless if you are junior or senior engineers, we both know that you want to prove yourself through code.

Posting a massive code review does not prove you are a reliable engineer.

Presumably, you can write a long piece of functional code without issues. But again, you do not know the norms on the team yet, and the massive amount of code draws a large target for code critics. Your code may not be necessarily wrong, but the nitpicking comments are just noise to slow you down.

It will become worse when: EngineerA supported your practice a year ago, and now he raises his argument again towards EngineerB for leading opposite practice. Your code review will become a battlefield.

Nah, please conduct a small change at a time, or break down your code change into smaller pieces.

5. Never make the same mistake

Let's say you have 1) read others' code reviews 2) finished the convention wiki 3) received feedbacks on your initial code reviews.

Make sure to note down all the mistakes you are aware of and avoid the same mistakes!

For example, in my world, common nitpickings are like:

• OMG don't ever go over 100 lines for a function!

20 public String functionA(...) {
21 ...
22 ...
.. ...
150
151 }

• Indentation matters, break your function parameters into multiple lines for readability

// don't
public String functionB(String param1, ServiceA param2, ServiceB param3, ServiceC param4, ServiceD param5) {}
// do
public String functionB(String param1,
                        ServiceA param2,
                        ServiceB param3,
                        ServiceC param4,
                        ServiceD param5)
{
    ...
}

You can avoid nits like these utilizing documentations. Don't make the same common mistake in your code review.

6. Earn trust before recommending a design change

Sure, we all have opinions.

I'd take 4-6 months to fully adjust to a team's coding dynamics before I push for design recommendations.

Your opinion matters, but to convince your peers, you have to earn their trust. Your team needs enough time to know you before they trust you with a significant code change.

Conclusion

If you forget everything above, take these away: 1) Learn the context, 2) Read the code, 3) Leverage code convention wiki, 4) Small steps, 5) Don't make the same mistake, 6) Earn trust before you thrive.

Happy learning!