CodeSOD: Allowed Savings

The CEO of Delia‘s company retired. They were an old hand in the industry, the kind of person who worked their way up and had an engineering background, and while the staff loved them, the shareholders were less than pleased, because the company was profitable, but not obscenely so. So the next CEO was a McKinsey-approved MBA who had one mission: cut costs.

Out went the senior devs, and much of the managers. Anyone who was product or customer focused followed quickly behind. What remained were a few managers handpicked by the new CEO and a slew of junior engineers- and Pierre.

Pierre was a contractor who followed the new CEO around from company to company. Pierre was there to ensure that nobody wasted any time on engineering that didn’t directly impact features. Tests? Wastes of time. Module boundaries? Just slow you down. Move fast and break things, and don’t worry about fixing anything because that’s your successors’ problem.

So let’s take a look at how Pierre wrote code. This block of PHP code was simply copy/pasted everywhere it needed to be used, across multiple applications.

foreach($alphaCheck as $checkField) {
  if($paramsArray[$checkField['field']]) {
    if($paramsArray[$checkField['field']]) {
      $invalidChars = false;
      $checkValue = trim(strtoupper($paramsArray[$checkField['field']]));
      $allowedChars = explode('|',"A|B|C|D|E|F|G|H|I|J|K|L|M|N|O|P|Q|R|S|T|U|V|W|X|Y|Z|-| |.|'");
      for($i=0; $i<strlen($checkValue); $i++) {
        if(!in_array($checkValue[$i],$allowedChars)) {
          $invalidChars = true;
        }              
      }            
      if($invalidChars) {
        $errorMsgs[] = $checkField['name'] . ' contains invalid characters';
      }            
    }          
  }        
}

This isn’t the worst approach to this problem I’ve seen in PHP, but the fact that this is just a copy/pasted blob, and worse- the $allowedChars may vary a bit in each place it’s copy/pasted is what makes it terrible.

Don’t worry. The new CEO only stayed for 18 months, got a huge bonus thanks to all the cost-cutting, and then left, taking Pierre along to the next company.

[Advertisement]
BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Source: Read More