14

I’m a senior frontend developer. My career is going great but I feel like I'm terrible at reviewing other people's code. There’s a much higher level of complexity that I can understand when its code I've written compared to reading other people.

I know this is true for everyone to some extent, but not sure if I’m worse than average. I also don't know how much care other people are putting into code reviews.

I’m also dyslexics which makes it hard to remember lots of variable names across multiple files. Anyone else feel this way? Or has anyone else had this issue but come up with a strategy that helps?

  • 3
    What tools are you using for code review? An IDE can help greatly. – Michael Jaros Aug 21 '19 at 11:53
  • Do you have the developer walk you through it first? We used to do that for code reviews of complex assembly language programs. – Laconic Droid Aug 21 '19 at 12:21
  • 6
    This is not a workplace.SE question. – Chris Aug 21 '19 at 17:30
  • Which language do you use? Which coding standards do you adhere to? – Benjamin Feb 11 at 15:23
  • 3
    This probably fits better in the software engineering stack exchange. – Jim Clay Feb 11 at 15:36
10

There's a golden rule of programming: Code is meant to be read by humans, and only incidentally executed by computers.

If you can't read the code? Then it's bad code.

"But maybe it's because I don't have the best memory, and I have to try to..."

No. Stop. As much as I love Internal Locus of Control, and finding ways to fix issues instead of blaming others... this really is an issue of bad code, because if a human can't easily read it, it doesn't matter how well the computer can execute it. Let me give an example:

// Code File #1:

Declaration of some variable named execptr;

// Code File #2:

if (execptr == null)
{
    startSvc();
}

... why is there some variable in code file #1 that's being referenced like that in Code File #2? Why is it named horribly? Why isn't the code in Code File #2 clean and self-documenting?

Compare this with:

// Code File #1:

Declaration of some variable named GlobalServiceInstance

// Code File #2

bool serviceInstanceIsRunning = (GlobalServiceInstance != null);
if (!serviceInstanceIsRunning)
{
    StartGlobalServiceInstance();
}

Notice the difference? The second example, I've named the variable better, I've used a temporary variable (serviceInstanceIsRunning) to document what's going on, etc.

Short Story: If you can't read the code, Say So! Say, "I don't think this code is very clear - I can't tell at a glance what it's even trying to do. Can you use better variable names, better function names, etc?"

| improve this answer | |
  • 3
    I fully agree. It helps if everybody reads Clean Code by Robert Martin. He does an excellent job explaining how to write readable code. – Benjamin Feb 11 at 16:35
  • Also you need to understand the code to catch incorrectly named variables, classes and methods – Thorbjørn Ravn Andersen Feb 11 at 18:32
  • Why are you using global variables? Why does file2 not call file1.GetGlobalServiceInstance ()? But upvote for a good answer :-) – Mawg says reinstate Monica Feb 12 at 11:28
4

There’s a much higher level of complexity that I can understand when it’s code Ive written compared to reading other people.

There's nothing wrong with that - everyone is the same. If it's not obvious to you what this code is doing, it's likely the next person to check it out will have the same reaction. Remember that code reviews can be a dialog, not just the reviewer shouting some fixes at the committer. Talk with him, decide if there's a better or clearer way of achieving the same thing, and see if you can work something out where you understand what's going on.

I’m also dyslexics which makes it hard to remember lots of variable names across multiple files.

If the code requires you to remember lots of variable names across multiple files to understand it, then it sounds like it's potentially rather bad code (possibly too tightly coupled to too much other code.) As such, it's a point you should call out on review, and discuss a clearer approach. I couldn't make head nor tail of code that required me to remember lots of variable names across multiple files either, and I'm not dyslexic.

I also don't know how much care other people are putting into code reviews.

That varies enormously, but I think it's fair to say it should be a lot of care. The code review process is an investment against technical debt - and that is a very worthwhile investment.

| improve this answer | |
1

If you review my code, and you say "I don't understand it", then there are two obvious possibilities: There's something wrong with my code, or there's something wrong with you.

With the right tools, you will review and add comments where either you think my code is wrong, or where you don't understand it. If my code is right but you think it's wrong, quite likely better comments are needed. Quite possibly it needs changing from "correct" to "obviously correct". And of course the same if you don't understand it.

I do expect you to be proficient in the language used. I don't write "smart" code, but I use what the language offers and expect you to understand it. If you don't, I can explain it to you, but I won't change the code in that situation. For example, we have some Swift code, and if you don't know what "if let" means or what a "defer" statement is, tough, you'll have to learn it.

So if it's me on the other side, just do your code review the best you can, and we'll sort it out.

| improve this answer | |

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .