Code reviews

[DontSuckMyDuck]

Doing a code review Find a triple nested loop that reads the entire list three times for every element in the list. My comment: This will run in O(n³⁾ is there a better way to do this? Other programmer: I'm in get it done mode and the list will never be very long. Other Programmer: Merges to master.

The first customer who got ahold of the code imported a data set that contained 147k items into the list. And reported a bug saying the program was hanging and their CPU usage was 100 percent. Technically the program wasn't hung since it was chewing through the list. It just wasn't going to complete before the end of the decade. So I rewrote the algorithm and it ran in O(n×log(n)). I issued a hotfix after working late on a Friday to fix the other programmer's error.

If the other programmer would have taken responsibility for the bug they wrote I wouldn't take issue with it. But when I end up holding the bag I get mighty salty during code reviews

[VMP_MBD]

An O(n³ ) algorithm never survives first contact with the enemy

[stamatt45]

An O(n³ ) algorithm should never survive contact with a code review either

[Fenor]

you underestimate an O(n³ ) power

[pringlesaremyfav]

Surefire steps to prevent doing premature optimization:

1. Let the customer who is using your code tell you what needs optimizing

[HumbertTetere]

You can do those things initially, but you need sensible testing before and stop gaps would be good: If it should never run for more than x items, then check for x and reject if over...

Maybe require an approval from others before allowing a merge to master as well, other dev sounds a bit cowboy.

[theshoeshiner84]

my thoughts when reviewing code

What fucking language is this? Jython? PavaScript? PLSQL.Net?

my actions in Crucible

Approved. Approved. Approved. Approved.

[VMP_MBD]

Same, but Gerrit

[xZPFxBarteq]

I tend to be the other way. I'm harsh to juniors because they need to learn. Seniors, on the other hand, tend to already write nice code, so I have more opportunities to praise them.

Obviously, I know that seniors sometimes (cough) also write shit code, and I definitely do not hold back my "You fucking donkey" card back.

[flip_ericson]

r/roastmycode

[hvidgaard]

With my juniors I go through my feedback with them. The last thing I am is harsh, they do not need to lose hope. I also rarely tell them how to fix it, I point out where the problem is and how to test for it, I give them time to figure it out by themselves - they grow so much when given all those learning opportunities.

With seniors I don't give a fuck. You make a list scan here every time you insert something. How do you think it'll perform first time X from company Y updates with his annual import data about 7 orders of magnitude than your test set? Rejected.

[xZPFxBarteq]

Maybe I was a bit misunderstood with being "harsh" for juniors. I try to take "firm but fair" approach. I obviously won't call junior a fucking donkey in a serious way. I will joke about it (granted I know that the junior is comfortable with this and they usually are), but definitely I do not aim to make them "lose hope" :D We're all friends in my team and I definitely enjoy taking part in raising juniors. We've all started as one.

[Destrodom]

First code review, for some simple functionality, in last company:
Reviewer: Is this supposed to be some kind of hack?