Yet more code smells? More? Isn’t 13th bad luck? We see several symptoms and situations that make us doubt the quality of our development. Let's look at some possible solutions. Most of these smells are just hints of something that might be wrong. They are not rigid rules. Previous Parts Part I Part II Part III Part IV Part V Part VI Part VII Part VIII Part IX Part X Part XI Part XII Let's continue... Code Smell 61 - Coupling to Classes Classes are handy. We can call them and invoke them any time. Is this good? Problems Coupling Extensibility Hard to mock Solutions Use interfaces or traits (if available). Use Dependency Injection. Favor Loose Coupling. Sample Code Wrong public class MyCollection { 
     public bool HasNext { get; set;} //implementation details 
     public object Next(); ////implementation details 
}

public class MyDomainObject sum(MyCollection anObjectThatCanBeIterated) {
 //Tight coupling 
}

//cannot fake or mock this method since it always expects an instance of MyCollection Right public interface Iterator { 
     public bool HasNext { get; set;}
     public object Next();
}

public Iterator Reverse(Iterator iterator) {
    var list = new List<int>();
    while (iterator.HasNext) {
       list.Insert(0, iterator.Next());
    }
    return new ListIterator(list);
}

public class MyCollection implements Iterator { 
     public bool HasNext { get; set;} //implementation details 
     public object Next(); ////implementation details 
}

public class myDomainObject sum(Iterator anObjectThatCanBeIterated) {
 //Loose coupling 
}

//can use any Iterator (even a mocked one as long as it adheres protocol) Detection We can use almost any linter to find references to classes. We should not abuse since many uses might be false positives. Tags Coupling Conclusion Dependencies to Interfaces make a system less coupled and thus more extensible and testable. Interfaces change less often than concrete implementations. Some objects implement many interfaces, declaring which part depends on which interface makes the coupling more granular and the object more cohesive. More info Coupling: The one and only problem Wikipedia When your code depends on an interface, that dependency is usually very minor and unobtrusive. Your code doesn’t have to change unless the interface changes, and interfaces typically change far less often than the code behind them. When you have an interface, you can edit classes that implement that interface or add new classes that implement the interface, all without impacting code that uses the interface. For this reason, it is better to depend on interfaces or abstract classes than it is to depend on concrete classes. When you depend on less volatile things, you minimize the chance that particular changes will trigger massive recompilation. Michael Feathers Code Smell 62 - Flag Variables Flags indicate what happened. Unless their name is too generic. Problems Readability Maintainability Coupling Solutions Use meaningful names Try to avoid flags. They generate coupling. Sample Code Wrong <?

function dummy() {

    $flag = true;

    while ($flag == true) {

        $result = doSomething();
        if ($result) {
            $flag = false;
        }
    }
} Right <?

function dummyFunction()
{
    $atLeastOneElementWasFound = true;

    while (!$atLeastOneElementWasFound) {

        $elementSatisfies = doSomething();
        if ($elementSatisfies) {
            $atLeastOneElementWasFound = true;
        }
    }
} Detection We can search all the code for bad-named flags. Tags Readability Conclusion Flags are widespread on production code. We should restrict their usage and use clear and intention revealing names. More Info Wikipedia ] What exactly is a name: part ii rehab If you lie to the compiler, it will get its revenge. Henry Spencer Code Smell 63 - Feature Envy If your method is jealous and you don't trust delegation, you should start to do it. TL;DR: Don't abuse your friend objects. Problems Coupling Low Reuse Low Testability Bad Responsibilities Assignment Bijection Fault Solutions Move method to the appropriate class. Sample Code Wrong class Candidate {

 void printJobAddress(Job job) {

   System.out.println("This is your position address");

   System.out.println(job.address().street());
   System.out.println(job.address().city());
   System.out.println(job.address().ZipCode());
 } 
} Right class Job {

 void printAddress() {

   System.out.println("This is your job position address");

   System.out.println(this.address().street());
   System.out.println(this.address().city());
   System.out.println(this.address().ZipCode());
  
  //We might even move this responsability directly to the address !
  //Some address information is relevant to a job and not for package tracking
 } 
}

class Candidate {
  void printJobAddress(Job job) {
    job.printAddress();
  }
} Detection Some linters can detect a sequential pattern of collaborations with another object. Tags Coupling Conclusion We should assign responsibilities according to real object and avoid abusing other objects protocol. mappers More info Refactoring Guru C2 Wiki Wikipedia We argue that design practices which take a data-driven approach fail to maximize encapsulation because they focus too quickly on the implementation of objects. We propose an alternative object-oriented design method which takes a responsibility-driven approach. Rebecca Wirfs-Brock Code Smell 64- Inappropriate Intimacy Two classes entangled in love. Problems Coupling Bad Responsibilities Assignments Bad Cohesion Class Interfaces too Public Maintainability Extensibility Solutions Refactor Merge . Replace Hierarchy With Delegation Sample Code Wrong class Candidate {

 void printJobAddress(Job job) {

   System.out.println("This is your position address");

   System.out.println(job.address().street());
   System.out.println(job.address().city());
   System.out.println(job.address().zipCode());
   
   if (job.address().country() == job.country()) {
        System.out.println("It is a local job");
   } 
} Right final class Address {
 void print() {
   System.out.println(this.street);
   System.out.println(this.city);
   System.out.println(this.zipCode);   
 } 
 
 bool isInCounty(Country country){
  return this.country == country;
}

class Job {
 void printAddress() {

   System.out.println("This is your position address");

   this.address().print());
   
   if (this.address().isInCountry(this.country()) {
        System.out.println("It is a local job");
   } 
 } 
}

class Candidate {
  void printJobAddress(Job job) {
    job.printAddress();
  }
} Detection Some linters graph class relations and protocol dependency. Analyzing the collaboration graph, we can infer rules and hints. Tags Coupling Conclusion If two classes are too related and don't talk much to others, we might need to split, merge or refactor them; Classes should know as little about each other as possible. More info C2 wiki Refactoring guru thecodebuzz.com No matter how slow you are writing clean code, you will always be slower if you make a mess. Code Smell 65 - Variables Named after Types Names should always indicate role. Problems Declarative Design for Change Coupling to accidental implementation Solutions Rename your variable according to the role. Sample Code Wrong public bool CheckIfStringHas3To7LowercaseCharsFollowedBy3or4Numbers(string string)
{
  Regex regex = new Regex(@"[a-z]{2,7}[1-9]{3,4}")
  var hasMatch = regex.IsMatch(string);
  return hasMatch;
} Right public bool CheckIfStringHas3To7LowercaseCharsFollowedBy3or4Numbers(string password)
{
  Regex stringHas3To7LowercaseCharsFollowedBy3or4Numbers = new Regex(@"[a-z]{2,7}[1-9]{3,4}")
  var hasMatch = stringHas3To7LowercaseCharsFollowedBy3or4Numbers.IsMatch(password);
  return hasMatch;  
} Detection This is a semantic rule. We can instruct our linters to warn us from using names related to existing classes; types o reserved words since they are too implementative. Tags Declarative Conclusion The first name we can come across is related to an accidental point of view. It takes time to build a theory on the models we are building using our . Once we get there, we must rename our variables- MAPPERS More info Whats is in a name xunitpatterns.com Credits This idea came from this tweet Types are essentially assertions about a program.  And I think it’s valuable to have things be as absolutely simple as possible, including not even saying what the types are. Dan Ingalls We are done for some time (not many). But we are pretty sure we will come across even more smells very soon! I keep getting more suggestions on , so they won't be the last! Twitter Send me your own!

Twitter

Refactoring: Remove Setters #CodeSmell

How to Find The Stinky Parts of Your Code [Part XII]

Buy My Book

How to Find The Stinky Parts of Your Code [Part XIII]

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

10 Predictions About the Future of Finance

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

110 Stories To Learn About Refactoring

3 Things Coding And Prose Writing Have In Common According To Cory House

4 Simple Habits For Engineering Teams To Beat Technical Debt

10 Predictions About the Future of Finance

106 Stories To Learn About Clean Code

100 Pieces of Programming Advice from the Book Clean Code by Robert Martin

110 Stories To Learn About Refactoring

3 Things Coding And Prose Writing Have In Common According To Cory House

4 Simple Habits For Engineering Teams To Beat Technical Debt

Light-Mode

Classic

Newspaper

Dark-Mode

Neon Noir

Minty

HN StartUps