Refactoring Legacy Code

InsightsToRefactor.md

Comments Code Smell

• Remove unnecessary comments.
• If the code is obvious, don’t write a comment.
• Don’t leave commented old code.
• Remove commented debugging var_dump, dump, echo, ..etc.

Best Practices:

• Follow Psr2: https://www.php-fig.org/psr/psr-2/
• Always use {} within if-statements :
  • one line if-statements some developers use it because it is simple, however, it is not readable and it’s easy to cause problems since only one empty line can break the condition and start crashing

if ( ! $isAdmin) 
     return false;

• Do not use magic numbers or magic strings:

if ($rooms < 200) {
    return false;
}

instead of use constant or variable:

$maxRoom = 200;
if ($rooms < $maxRoom) {
  return false;
}

• Do not use else-statements if you do not need to.
• Use a new array form [ ] instead of the old one array().
• Use === operator instead of == unless it is important to not check for the dataType.
• Use type Hint instead of manual validation:

public function profile($age, $name) 
{ 
  if (!is_int($age)) { 
    throw new \Exception('Provided age not valid');
  }

  if (!is_string($name)) {
    throw new \Exception('Provided name not valid');
  }
 //some logic
}

instead of use :

public function (int $age, string $name)
{ 
    //some logic 
}

• It is always a good idea to give public methods short descriptive names. It is ok for private methods to have longer names as they have a limited scope
• Remove unused methods from your classes and nameSpaces
• Use prefix is/has with functions that return boolean ex: isAdmin($user), hasPermission($user).
• Always use access modifiers in class methods and properties.
• Organize classes methods where public methods are on the top.
• Long Methods and Classes Code Smell 'The perfect method should be between 4 to 20 lines'
• (Dry) Duplicated Methods in the Same or Different Class Code Smell
• Don’t over plan your code.
• Don’t try to cover a case that likely has 1% chance to happen in the future.
• Always apply the single responsibility concept to your classes.
• Never refactor a production code that does not have unit tests

Exception handling guidelines/practices:

1. Don't catch Exception unless that's all you're having thrown to you.
2. Don't declare that you throw Exception ever.
3. Both rules #1 and #2 apply to RuntimeException as well.
4. Don't catch Throwable if you want to continue breathing.
5. Rule #4 applies to Error and any subclasses of Error as well.
6. Don't catch, log and rethrow.
7. Familiarise yourself with the standard RuntimeException subclasses (IllegalStateException, IllegalArgumentException, UnsupportedOperationException, IndexOutOfBoundsException), and use them in preference to creating your own runtime exception class.

• For example, if the problem is that an object reference (or "pointer") which you didn't expect to be null is in fact null, why not throw a NullPointerException? If you explicity throw any RuntimeException in a method, document it in the method's @throws Javadoc like you would a checked exception.