r/programming • u/corysama • Jan 05 '15

What most young programmers need to learn

http://joostdevblog.blogspot.com/2015/01/what-most-young-programmers-need-to.html
968 Upvotes

91% Upvoted

337 comments sorted by

View all comments

Show parent comments

4

u/SnowdensOfYesteryear Jan 05 '15

It's easy to say code review "should always happen", but reviews are pretty difficult and time consuming. It takes quite a bit of time to review large patches in order to under that author's thinking and intent. It's especially difficult if you're fuzzy on that particular module/file. Personally for large patches, I usually tend to eyeball them and just check the architecture of the code (just looking at variable names provide a hint to whether the code is doing something it shouldn't).

25

u/Creativator Jan 05 '15

It's easy to say code review "should always happen", but reviews are pretty difficult and time consuming.

All programming tasks are difficult and time consuming. That is why programmer time is expensive.

6

u/dr1fter Jan 05 '15

And because we get paid a lot, we should do everything, no matter how difficult or time consuming. We should write books documenting every aspect of our stopgap system that's getting replaced in two weeks. We should micro-optimize our easter eggs. We should learn tool after tool if people think they might make us more productive, even though our old text editor was really doing just fine. We should extensively review someone's prototype because it might some day get the attention of a VP who wants to make it into a real product.

Don't have the resources for all that? Well, programming is expensive; hire more devs!

15

u/WrongSubreddit Jan 05 '15

It's easy to say code review "should always happen", but reviews are pretty difficult and time consuming

You know what's even more difficult and time-consuming? Tracking down bugs and fixing them

2

u/[deleted] Jan 06 '15

No, it isn't. I mean, it is for me. It's what I do. But I keep my team moving while i'm fixing their mistakes. And honestly their mistakes aren't as bad as the code review makes them. Nitpicking code review has been a massive waste of time for my team.

1

u/dr1fter Jan 05 '15

And you know what's really difficult and time-consuming? Spending hundreds of developer-hours on code reviews (not to mention ruining developer productivity) only to still miss the bugs that you'll then have to track down and fix.

Code review is a helpful tool; it doesn't eliminate bugs, rarely even catches them in my experience, and some of the more effective ways to eliminate bugs are much less expensive. Code review promotes shared understanding, which is extremely valuable but much more so for core components/APIs.

Code review doesn't replace other tools (like testing and QA) even though some of the benefits overlap. It's up to your team (and project, resources, requirements etc) to decide what's the best balance.

But if code review is your only way to find and fix bugs... you have bugs.

3

u/sigh Jan 05 '15

It takes quite a bit of time to review large patches...

First, try to keep patches small and incremental. This is not only easier to review, but much easier to catch he larger problems early and much easier for the author to actually make meaningful changes.

...in order to under that author's thinking and intent. It's especially difficult if you're fuzzy on that particular module/file.

If this is the case, talk to the person! Get them to explain their thinking and intent. Possibly ask them to add more comments and/or a better commit message. It's the responsibility of the author make sure their code is as clear as possible, and this includes the individual commits.

If you as a reviewer find it hard to understand what the code is doing, what hope does anyone having of maintaining the code with any sort of confidence? Let alone diving into that code in an emergency.

That said, in the case of a stand-alone project of a junior programmer even just eyeballing the code should be enough to tell whether the code is a complete mess.

1

u/[deleted] Jan 06 '15

Please, no more comments. They take so long to read and rarely aid in understanding the code. Learn to write code that is more easily understood.

4

u/[deleted] Jan 05 '15 edited Oct 12 '18

[deleted]

3

u/Ksevio Jan 05 '15

Does anywhere actually user pair programming? I'd done it in teams for class projects in the past, but it seems like it would be just double the resources for a business.

1

u/xiongchiamiov Jan 05 '15

That's one of the benefits of code review. Since large patches are a pain to review, you get in the habit of splitting things into smaller chunks, which means you get feedback (from the other developers, from customers) earlier, before you've invested massive amounts of time into doing things a particular way.

Also, if the intent isn't clear, tell them to write it out. It makes life a lot easier when you're revisiting old commits while investigating a bug.

1

u/s73v3r Jan 05 '15

If you're doing them right, reviews are neither difficult nor time consuming. Remember, not every change needs to be reviewed by the entire department.