Now that Elon Musk has bought Twitter - now the Pit edition (Part 1)

The BBQ Pit
1556

I tried the link, and get a ‘this page does not exist’ from Twitter. Can you check it?

There are a lot of pretty fundamental errors you can spot in a code review - which is why code reviews are a thing. Do you also think the practice of giving a coding test to potential hires is a useless thing? Ater all, what can you learn from a few lines of code, right?

As someone who has literally done hundreds of code reviews, let me give you a very short list of examples of red flags you can find in a review:

Not handling exceptions properly. If a method throws an exception, you need to handle that exception when you call that method. Unhandled exceptions can cause all kinds of problems down the road.

Also, it’s a no-no to just handle the generic exception:

catch (Exception e) {}

.Some functions can throw multiple exception types, and you should handle them specifically. Your job as a pro is to make sure you understand what exceptions a library call can throw (by reading the docs) and make sure you handle each one, and document the shit out of it.
:

//Make sure the rocket doesn't explode when it passes through the vertical and the cosine of the angle returns 0.
catch (DivideByZeroException  e) {}

Magic constants. It’s not uncommon to find lines like,

If (var == 3.1415)
{
}

Instead of creating a constant called PI, setting it to the value, then testing:

#define PI 3.1415926;

function CheckForSpecialValue(var)
{ 
  if(var == PI)
  {
  }
}

PI is obvious, but you’d be shocked how many times you find ‘magic numbers’ that aren’t clear at all. Some number that means something, cut and pasted a hundred times through the code instead of setting a constant.

Useless comments. Comments are not needed for obvious things, but are needed when code does something that’s not common, or is opaque enough to warrant a description. Weak programmers won’t comment, and when told to will often comment like this:

  A =  B / C;   // Divide B by C and set to A

Or a function with a comment like this:

//Gets port address
function getPortAddress()
{
}

In the meantime, a crazy test goes undocumented or has a lame comment

//Do the test
if ((A && B || C)  && (!C && D)  || (A && D))
{
  // Do something
}
else if (!A && !D) || C) 
{
  // Do something else
}

For that matter, long chains of complex booleans can almost always be written differently and more understandably, and the boolean logic can usually be reduced through algabraic reduction (de Morgan’s theorem, etc).

I did a code review just like that once. The developer wrote a boolean expression about three lines long. I told him he needed to reduce it, and he didn’t know what I was talking about. I mentioned a couple of ways, and he lookd blank and said he’d never heard of any of it. I know for a fact that this stuff is taught in any CS faculty worthy of the title, so I assumed he got a low mark in that class and didn’t absorb the material.

I once did a code review for a developer who was writing a handler for a couple of variables. His code looked like this:

If (var1 == 1) 
{
  if (b == 1)
    callfunction(var1);
  if (b == 2)
    callfunction(b);
  if (b == 3)
   callfunc2(b);
}
else if (var1 == 2)
{
if (b == 1)
    callfunction2(var1);
 if (b == 2)
    callfunction2(b);
 if (b == 3)
   callfunc3(b);
}
else if (var1 == 3)
{
  ...etc
}

Lots more like that. These aren’t ‘formatting’ errors. They aren’t trivial. They speak to a lack of clear thinking, or laziness, or failure to adhere to standards, or poor education. Youy’ll never see a really good programmer making errors like this.

Code can read almost like an IQ test. Convoluted spaghetti code vs elegant code that is clear, readable and performant often means, in my experience, that the guy writing the spaghetti code either isn’t that smart, or doesn’t care, or had a lousy education.

If I were Musk and was looking to sort out devs by code review, the things above (and many more) would be red flags. I would then ask the developer to defend the way the code was written, and if they can’t… sayanora.

In a normal time for a normal employee review that wouldn’t be the case, because you’d want to try to educate them and get them doing things the right way first. But in the context of having to lay off half the coding work force in a fairly quick manner, this would be a reasonable way to go, given the difficulty of any method you chose.