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

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.

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.

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.

Conclusion

We should assign responsibilities according to real object mappers and avoid abusing other objects protocol.

More info

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

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.

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.

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 MAPPERS. Once we get there, we must rename our variables-

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 Twitter, so they won't be the last!

Send me your own!

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

Previous Parts

Code Smell 61 - Coupling to Classes

Problems

Solutions

Sample Code

Wrong

Right

Detection

Tags

Conclusion

More info

Code Smell 62 - Flag Variables

Problems

Solutions

Sample Code

Wrong

Right

Detection

Tags

Conclusion

More Info

Code Smell 63 - Feature Envy

Problems

Solutions

Sample Code

Wrong

Right

Detection

Tags

Conclusion

More info

Code Smell 64- Inappropriate Intimacy

Problems

Solutions

Sample Code

Wrong

Right

Detection

Tags

Conclusion

More info

Code Smell 65 - Variables Named after Types

Problems

Solutions

Sample Code

Wrong

Right

Detection

Tags

Conclusion

More info

Credits