CODEWITHSUNDEEP
oopdependenciesanti-patterns
Tom B
Tom B

Reputation: 2923

Is there a common name for this code smell?

I refer to it as the "delivery boy". I've seen several variants of it but the issue is that a class has dependency for the sole purpose of passing it on to collaborators and never using the dependency itself.

(I'm using PHP because it's what I'm most familiar with but this is language agnostic)

class Dependency{}

class B {
    public function setDependency(Dependency $dependency) {
        //...
    }
}

class A {
    private $b;
    private $dependency;

    public function __construct(Dependency $dependency, B $b) {
        $this->dependency = $dependency;
        $this->b = $b;
    }

    public function foo() {
        $this->b->setDependency($this->dependency);
    }
}

Probably the most common variant I see in the wild is abusing inheritance for this purpose, having a property in the parent class which exists so that the child classes have access to the dependency even if the parent class never actually uses the dependency itself.

class Dependency{}



class A {
    protected $dependency;

    public function __construct(Dependency $dependency) {
        $this->dependency = $dependency;
    }
}

class B extends A {
    public function foo() {
        $this->dependency->bar();
    }
}

I see this in code far more than I'd like and it doesn't make me very happy! I just wondered if there was a name for this so that I can link people to reading materials on why it's a bad idea. As it stands, I don't know what to search for!

Upvotes: 1

Views: 349

Answers (2)

Alex
Alex

Reputation: 1017

The problem related to inheritance in the second snippet looks like to me "Broken Hierarchy". This smell occurs when the base class and its derived class do not share an IS-A relationship. It is very common to find code that uses inheritance just for convenience (for reuse) and not because it makes sense to have a hierarchy where the participating classes are are related (by IS-A relationship).

(I borrowed the smell terminology (i.e. Broken Hierarchy) from the book "Refactoring for software design smells")

Upvotes: 0

Erik Funkenbusch
Erik Funkenbusch

Reputation: 93444

I'm not aware of any name, but I kind of like Delivery boy... though I suppose some might consider the name borderline offensive.

Typically this problem is solved with either Dependency Injection or a Service Locator, although way too many people use Singleton for this (inappropriately).

I'm not familiar enough with PHP to know if PHP offers a real DI solution (as opposed to poor man's DI), but I think a service locator would be acceptable if there isn't (even though service locator is often a code smell in itself).

Upvotes: 1

Related Questions