Code Smells Are a Classic. It smells because there are likely many instances where it could be edited or improved. Most of these smells are just hints of something that might be wrong. Therefore, they are not required to be fixed per se… (You should look into it, though.) Previous Code Smells You can find all the previous code smells (Part i - XXX) here Let's continue... Code Smell 151 - Commented Code Beginners are afraid to remove code. And many seniors too. TL;DR: Don't leave commented code. Remove it. Problems Readability Dead Code Lack of Coverage Lack of Source Version Control Solutions Remove commented code Implement Source Version Control Refactorings Refactoring 005 - Replace Comment with Function Name Context When debugging code we tend to comment on code to see what happens. As a final step, after all our tests pass, we must remove them following clean code practices. Sample Code Wrong function arabicToRoman(num) {
  var decimal = [1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1];
  var roman = ['M', 'CM', 'D', 'CD', 'C', 'XC', 'L', 'XL', 'X', 'IX', 'V', 'IV', 'I'];
  var result = '';
  
  for(var i = 0; i < decimal.length; i++) {
    // print(i)
    while(num >= decimal[i]) {
      result += roman[i];
      num -= decimal[i];
    }    
  }
  // if (result > 0 return ' ' += result)
  
 return result;
} Right function arabicToRoman(arabicNumber) {
  var decimal = [1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1];
  var roman = ['M', 'CM', 'D', 'CD', 'C', 'XC', 'L', 'XL', 'X', 'IX', 'V', 'IV', 'I'];
  var romanString = '';
  
  for(var i = 0; i < decimal.length; i++) {
    while(arabicNumber >= decimal[i]) {
      romanString += roman[i];
      num -= decimal[i];
    }    
  }
  
 return romanString;
} Detection Semi-Automatic [x] Some machine learning analyzers can detect or parse comments and guide as to remove them. Tags Comments Conclusion We need to remove all commented-out code. Relations Code Smell 75 - Comments Inside a Method Code Smell 05 - Comment Abusers Credits Photo by on maxim bober Unsplash Don’t document the problem, fix it. Atli Björgvin Oddsson Software Engineering Great Quotes Code Smell 152 - Logical Comment Temporary hacks might be permanent TL;DR: Don't change code semantics to skip code. Problems Readability Non-Intention Revealing Solutions If you need a temporary hack, make it explicit Rely on your source control system Context Changing code with a temporary hack is a very bad developer practice. We might some temporary solutions and leave them forever. forget Sample Code Wrong if (cart.items() > 11 && user.isRetail())  { 
  doStuff();
}
doMore();
// Production code

// the false acts to temporary skip the if condition
if (false && cart.items() > 11 && user.isRetail())  { 
  doStuff();
}
doMore();

if (true || cart.items() > 11 && user.isRetail())  {
// Same hack to force the condition Right if (cart.items() > 11 && user.isRetail())  { 
  doStuff();
}
doMore();
// Production code

// Either if we need to force or skip the condition
// we can do it with a covering test forcing
// real world scenario and not the code

testLargeCartItems() {}

testUserIsRetail() {} Detection Semi-Automatic [x] Some linters might warn us of strange behavior. Tags Comments Conclusion Separation of concerns is extremely important in our profession. Business logic and hacks should always be apart. Relations Code Smell 151 - Commented Code Credits Photo by on Belinda Fewings Unsplash Thanks, @ for this tip Ramiro Rela You might not think that programmers are artists, but programming is an extremely creative profession. It's logic-based creativity. John Romero Software Engineering Great Quotes Code Smell 153 - Too Long Names Names should be long and descriptive. How Long? TL;DR: Names should be long enough. No longer. Problems Readability Cognitive Load Solutions Use names related to the MAPPER Context We used during the 50s and 60s due to space and time constraints. very short names This is no longer the case in modern languages. Sometimes we get too excited. Naming is an art and we should be cautious with it. Sample Code Wrong PlanetarySystem.PlanetarySystemCentralStarCatalogEntry

// Redundant Right PlanetarySystem.CentralStarCatalogEntry Detection Semi-Automatic [x] Our linters can warn us with too long names. Tags Bloaters Naming Conclusion There are no on name length. hard rules Just Heuristics. Relations Code Smell 33 - Abbreviations More Info What exactly is a name? — Part I: The Quest What exactly is a name? — Part II: Rehab Long and Short of Naming Credits Photo by on Emre Karataş Unsplash Many people tend to look at programming styles and languages like religions: if you belong to one, you cannot belong to others. But this analogy is another fallacy. Niklaus Wirth Software Engineering Great Quotes Code Smell 154 - Too Many Variables You debug code and see too many variables declared and active TL;DR: Variables should be as local as possible Problems Readability Code Reuse Solutions Extract Method Remove unused variables Refactorings Refactoring 002 - Extract Method Context Our code should be dirty when programming and writing test cases fast. After we have good coverage we need to refactor and reduce methods. Sample Code Wrong <?

function retrieveImagesFrom(array $imageUrls) {
  foreach ($imageUrls as $index => $imageFilename) {
    $imageName = $imageNames[$index];
    $fullImageName = $this->directory() . "\\" . $imageFilename;
    if (!file_exists($fullImageName)) {
      if (str_starts_with($imageFilename, 'https://cdn.example.com/')) {
          // TODO: Remove Hardcode
          $url = $imageFilename;
          // This variable duplication is not really necessary 
          // When we scope variables        
          $saveto= "c:\\temp"."\\".basename($imageFilename);
          // TODO: Remove Hardcode
          $ch = curl_init ($url);
          curl_setopt($ch, CURLOPT_HEADER, 0);
          curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
          $raw = curl_exec($ch);
          curl_close ($ch);
          if(file_exists($saveto)){
               unlink($saveto);
          }
          $fp = fopen($saveto,'x');
          fwrite($fp, $raw);
          fclose($fp);
          $sha1 = sha1_file($saveto);
          $found = false;
          $files = array_diff(scandir($this->directory()), array('.', '..'));
          foreach ($files as $file){
              if ($sha1 == sha1_file($this->directory()."\\".$file)) {                         
                  $images[$imageName]['remote'] = $imageFilename;
                  $images[$imageName]['local'] = $file;
                  $imageFilename = $file;
                  $found = true;
                  // Iteration keeps going on even after we found it
              }
          }
          if (!$found){
            throw new \Exception('We couldnt find image');
         }
        // Debugging at this point our context is polluted with variables
        // from previous executions no longer needed
        // for example: the curl handler
  } Right <?php

function retrieveImagesFrom(string imageUrls) {
  foreach ($imageUrls as $index => $imageFilename) {
    $imageName = $imageNames[$index];
    $fullImageName = $this->directory() . "\\" . $imageFilename;
    if (!file_exists($fullImageName)) {
        if ($this->isRemoteFileName($imageFilename)) {
            $temporaryFilename = $this->temporaryLocalPlaceFor($imageFilename);
            $this->retrieveFileAndSaveIt($imageFilename, $temporaryFilename);
            $localFileSha1 = sha1_file($temporaryFilename);
            list($found, $images, $imageFilename) = $this->tryToFindFile($localFileSha1, $imageFilename, $images, $imageName);
            if (!$found) {
                throw new \Exception('File not found locally ('.$imageFilename.'). Need to retrieve it and store it');
            }
        } else {
            throw new \Exception('Image does not exist on directory ' . $fullImageName);
        }
    } Detection Automatic [x] Most Linters can suggest use for long methods. This warning also hints us to break and scope our variables. Tags Bloaters Conclusion is our best friend. Extract Method We should use it a lot. Relations Code Smell 03 - Functions Are Too Long Code Smell 107 - Variables Reuse Code Smell 62 - Flag Variables Credits Photo by on Dustan Woodhouse Unsplash Temporary variables can be a problem. They are only useful within their own routine, and therefore they encourage long, complex routines. Martin Fowler Software Engineering Great Quotes Code Smell 155 - Multiple Promises You have promises. You need to wait. Wait for them all TL;DR: Don't block yourself in a sorted way. Problems Indeterminism Performance bottleneck Solutions Wait for all promises at once. Context We heard about semaphores while studying Operating Systems. We should wait until all conditions are met no matter the ordering. Sample Code Wrong async fetchOne() { /* long task */ }
async fetchTwo() { /* another long task */ }

async fetchAll() {
  let res1 = await this.fetchOne(); 
  let res2 = await this.fetchTwo();
  // they can run in parallel !!  
} Right async fetchOne() { /* long task */ }
async fetchTwo() { /* another long task */ }

async fetchAll() {
  let [res3, res4] = await Promise.all([this.fetchOne(), this.fetchTwo()]);
  //We wait until ALL are done
} Detection Semi-Automatic [x] This is a semantic smell. We can tell our linters to find some patterns related to promises waiting. Tags Performance Conclusion We need to be as close as possible to real-world business rules. If the rule states we need to wait for ALL operations, we should not force a particular order. Credits Thanks for the idea Twitter Photo by on Alvin Mahmudov Unsplash JavaScript is the only language that I'm aware of that people feel they don't need to learn before they start using it. Douglas Crockford Software Engineering Great Quotes Next article: 5 more code smells.

Too Long; Didn't Read

Making security fun one episode at a time

How to Find the Stinky Parts of Your Code [Part XXXI]

Too Long; Didn't Read

Maximiliano Contieri

About Author

TOPICS

THIS ARTICLE WAS FEATURED IN...

How to Find the Stinky Parts of Your Code [Part XXXI]

Too Long; Didn't Read

Maximiliano Contieri

About Author

TOPICS

THIS ARTICLE WAS FEATURED IN...

RELATED STORIES