Learning to Review Code

This week's open source lab has the students reviewing each other's code on GitHub.  I've purposely given every student the same programming problem so that they would a) all have the same knowledge about the problem space; b) get to see how another person might solve it in a totally different way.  Above all it's a chance to gain some experience working on both sides of a GitHub Issue before I send them out into the wider world for Hacktoberfest in a couple of weeks.

I've spent some time this morning reading through the blog posts that are already posted to Telescope.  There are some great insights being shared, and I wanted to say something about the various themes I've seen emerging.

The difficulty of estimation in software development

I did start my release 0.1 one night before the due date and did not recognize how rusty I am.

Despite what anyone tells you (including Bill Gates), you can’t do software development effectively at the last minute.  No one can.  Writing code of any length necessarily involves hitting issues you don't expect, the need to do research and learn skills you don't have, and tripping over problems that you can’t see until you’re well into the code.

The trick to software is to give yourself enough time to succeed.  I see a lot of self-sabatoge, where students set themselves up for failure.  You need time to learn, time to debug, time to walk away in frustration, and then time to come back and keep working.  Most of all, you need time to communicate with your partner, team, and community.  Code always involves people, too.

Students often think they can’t code something, but in reality it's just that they can’t code it in the tiny amount of time they've allotted themself.  You're capable of a lot if you give yourself a chance:

At first I was panicking a lot. The problem was that I didn't know the how.  I didn't know the steps. I didn't know where to look. But really - I  did. I just thought it was too much. So I wrote down a checklist and  went one by one. The more progress I made - the easier it became.

Reading other people's code is a skill to be learned

Reviewing someone else's project and having mine reviewed was.... weird...I found their code pretty different.
At first, the idea of reviewing someone else's code is daunting to me. Everyone uses different logic and different tools to implement that logic.
To me, testing and reviewing someone’s code isn’t easy as it involves multiple skills such as communication, knowledge of the programming language/ library, coding, and an open heart to learn new things.

When you mostly read your own code, it can be a lot to jump into someone else's code, and by extension, someone else's head.  We think about problems differently, use different approaches, and favour different styles.

It's useful to recognize that this isn't something that will neccesarily feel natrual or be easy at first.  It takes time to become comfortable being uncomfortable in other people's code.  It takes time, but it will happen.  Keep going.

If you let it, reading other people's code can open your mind to new ideas

It was interesting to look at someone else's way to tackle the same problem
She even checked for empty path input which I did not think about until reading her code.
I was able to learn a lot just from watching my partner code
Seeing how someone else does the same project but with different lines of thinking or logic is really cool.
Even though this is a fairly simple program there are many approaches different people take

If we approach another person's code with an open mind, we can often learn a lot. It's easy to go into a review and only look for the problems.  This is an important part of review, but not the whole story.  Code review is also how we share knowledge in a community, and where we learn about new techniques, tools, algorithms, and approaches. If we can let go of our ego and go into a code review with a willingness to learn, there's often an equal benefit for the person doing the review as there is for the one getting the review.

It won't necessarily feel good to have people critique your code

nervous to have someone review my code
the thought...was horrifying

Why would I ever want to showcase how little I know?  Why would I want to expose my code (and myself!) to ridicule from programmers who are so much better than me?

We put a lot of trust in our reviewers to treat us with respect, and it's imporant that we remember this whenever we are asked to do a review.  Code review isn't only about code: there's always a person behind it.

Putting yourself out there won't always feel good, and yet...

But code review helps me write better code

During the testing and reviewing for her code, I have encountered both minor and major issues.
she was able to find issues that I never realized before
Some things that I was not able to see in my code were found by my classmate and i was able to resolve them
It was interesting to see the obvious things I was missing
The feeling of reading others' codes is amazing, you will not only experience various methods to deal with the same problem, but you will also figure out whether the way you write your codes is optimum or not...I would say, if I did that project alone, I could never find those errors

It's amazing what another set of eyes can see, or how someone running my code on a different operating system can reveal assumptions I was making!  Even after spending hours and hours staring at this code I still miss things that a colleague can spot immediately.

Maybe I'm a good programmer on my own, but I become a great programmer when others are willing to help me review and fix my work.

Other interesting take-aways

the -o or --output option specified in the help section has not been applied yet. I solved this problem by commenting this option out cause I intend to implement that feature in the next version of the tool.

This is a brilliant approach.  Often the best way to fix a bug is to remove the entire feature until it's ready.  Even better than commenting it out would be to move that feature code onto its own branch in git, and keep it out of the main branch until it's ready to ship.

Sometimes, we have a tendency to stick to certain methods to deal with the problems depending on who are your instructors or what kind of documents you follow. This means if the person you are giving feedback to has a different way of writing code, compared to yours, it would be very confusing for you and you might end up having no idea what are they writing and what is the purpose of that line of codes.

I thought this was insightful.  We are all prone to confuse what's familiar with what's correct: "I do it like this because that's how I learned."  What if there's a better way? I think expanding what we know by reading lots of other code helps to mitigate this risk.

...his stylesheet code blew me away and I borrowed his module generateHtmlTemplate.js with properly citing it as his work.  That is the beauty of Open Source like this at Seneca.

I love this.  What would be cheating in another class is exactly the right thing to do here.  Even better would be to take that code and extract it into its own open source library that can be included more easily.  We'll get to that later.

Through communication on Slack and video call on discord we were able to successfully run each other's code.

I thought it was useful to call out how code review can be done both async and sync, and how different tools might be needed.  Talking in a GitHub Issue is fine for some things, but often, hopping into a call on Teams or Zoom, or talking with higher bandwidth in Slack or Discord can sort things out more quickly.  Don't be afraid to use the most appropriate tools.

Having said that, another point worth making is that taking review/discussion to private vs. public spaces is exclusionary.  How does a community learn from and contribute to your work if it all happens in DMs or private rooms?  If you do take a detour into a private meeting, make sure to report back on what was discussed, and which decisions were made (and why) in a public place later (e.g., in the GitHub Issue):

We communicated on problems mostly in slack, but forgot to make issues  for them. So we had solved them before even creating an issue. We had to  make issues later
Show Comments