Is this a good example of using the Dependency Inversion Principle?

Question

So I have a high level module, that uses a low level module (I intend to use this example for language agnostic explanatory purposes, so I removed access modifiers, getters & setters etc.):

Class SalariesHandler
{
    // properties…
    Printer printer;

    SalariesHandler(Printer printer..)
    {
        this.printer=printer;
    }
}

And then I invert the dependency between Printer and SalariesHandler like so:
Printer gets its SalariesHandler dynamically:

Class Printer
{
    PrintSalary(int salaryID)
    {
       ..
       ISalariesHandler salariesHandler = SalariesHandlerFactory.GetInstance();
       Salary salary = salariesHandler.GetSalary(salaryID);
       Print(salary.AsPrintable());
       ..
    }
}

And the factory is implemented as follows:

Class SalariesHandlerFactory
{
    ISalariesHandler GetInstance(){
        String currentSalariesHandlerType=getCurrentSalariesHandlerType(Settings settings);

        Switch (currentSalariesHandlerType) {
            Case “version1”: return new SalariesHandlerVersion1(); break;
            Case “version2”: return new SalariesHandlerVersion2(); break;
            ..
        }
    }
}

So finally, is this a good example of DIP?

Answer 1

No, I don't think it is a good example (at least not how it is implemented here). Let me explain why.

First, you get rid of the dependency SalariesHandler -> Printer - that is fine. The standard approach would be to introduce an interface IPrinter, but in this case, an even simpler solution is possible.

The code already shows how this can be accomplished: salariesHandler.GetSalary(salaryID) provides a Salary object, which has an AsPrintable() method, which delivers something a Printer can process. In case the method PrintSalary would be placed on a higher level like a controller class, both existing classes are already decoupled, they can be used and tested on their own, and maybe placed in separated black-box libraries.

However, when PrintSalary is misplaced inside the Printer object, it depends on SalariesHandlerFactory, which depends on all concrete derivations of ISalariesHandler. By calling GetInstance() directly inside Printer (instead of using something like constructor injection), Printer becomes dependent on all those derivations, too, for no apparent reason. Note that, even if constructor injection would have been used, it is pretty mystical what purpose the factory has, since all dependencies were already eliminated at that time.

The point of the DIP is to make "high level modules" (like SalariesHandler) not depend on "low level modules" like Printer. But that does not mean to make "low level modules depend on high level modules", that's a misconception probably caused by taking the word "inversion" too literally. It means to make both the low level and high level modules depend on abstractions (see Wikipedia, they use exactly this wording). Here, the abstraction on which SalariesHandler and Printer could both depend is something general which can be printed - for example, a format string, an html string, a list or table of strings, an abstract IPrintable or IReportPage object, whatever AsPrintable() delivers.

"Dependency inversion" is not an end in itself, it is a means to an end. Don't forget about the actual goal, which is usually easier reusage and testing. When the same goal can be accomplished by simpler means and "Dependency elimination" instead of "inversion", there is no need to artificially introduce an additional "inverted dependency" again.

Answer 2

Untested code

You clearly didn't compile this code or do any sanity checking on it before asking this question.

• getCurrentSalariesHandlerType(Settings settings) cannot possible be syntactically valid where you've used it.
• ISalariesHandler GetInstance() is not a static method, yet you do call it statically as SalariesHandlerFactory.GetInstance()
• Your SalariesHandler class doesn't implement ISalariesHandler but is still implied to implement said interface, based on your usage

More often than not, patterns and implementations can fail or succeed based on how it shapes the code around them, and your code is malformed regardless of the pattern you use.
It's hard to actually review code that hasn't passed any sanity check, as all the issues are both detracting and distracting from the potential validity of what you do want to discuss.

---

IOC and DI misinterpretations

First of all, dependency inversion is an extension of inversion of control, but you seem to have undone your inversion of control by "inverting" your dependency. Mind the quotes, because I think you misunderstood its purpose and implementation.

Based on its existence, the Printer class comes across as being intentionally kept separate from the salary classes, which is good SRP.

The original SalariesHandler (first code example) is a good way of doing inversion of control. The salary handler doesn't get to choose which printer it uses, that control is given to the consumer of the SalariesHandler. That is a straightforward IOC implementation.

But then you "invert" the dependency (what you did is not what dependency inversion is) and undo all that work. You hardcode SalariesHandlerFactory into the Printer. That throws up red flags for several reasons:

• Hardcoding types is the antithesis of inversion of control. You're specifically making it impossible for the consumer of Printer to choose the ISalariesHandler it wishes to use
• Your factory itself similarly refuses to give any control to the consumer, instead relying on some magic getCurrentSalariesHandlerType(Settings settings) decision (which is also not valid syntax, by the way). You don't specify where Settings comes from, but even if that is somehow controlled by the consumer, it's being done in a very indirect way and these classes themselves still don't follow IOC principles.
• This printer should really be called SalaryPrinter if it's going to hardcode a dependency on salary-related classes
• Your SRP separation has been effectively undone, as the printer is inevitably tied to salaries.

Additionally, there are significant code quality issues (e.g. using a stringly-typed currentSalariesHandlerType in a switch case).

So no, this is not a good example of anything.