A best practice is a method or technique that has consistently shown results superior to those achieved with other means, and that is used as a benchmark. In addition, a “best” practice can evolve to become better as improvements are discovered
Best practices are used to maintain quality as an alternative to mandatory legislated standards and can be based on self-assessment or benchmarking
Best can change, much like a theory but it’s only as developments change. In computing a new best practice rarely contradicts an old one.
Best practices exist and they exist for a reason, because they are consistently shown to be better than alternatives. While it’s not always possible to follow them fully, this should not be a reason to ignore them persistently or actively employ bad practices. Bad practices are always bad practices.
(…) The question itself wasn’t especially remarkable (…) but compiler author and programming legend Eric Lippert (…) say “DI is basically a bad idea.” To elaborate, he said:
There is no killer argument for DI because DI is basically a bad idea. The idea of DI is that you take what ought to be implementation details of a class and then allow the user of the class to determine those implementation details. This means that the author of the class no longer has control over the correctness or performance or reliability of the class; that control is put into the hands of the caller, who does not know enough about the internal implementation details of the class to make a good choice.
I was floored. Here we have one of the key authors of the C# compiler saying that the “D” in the SOLID principles was a “bad idea.” I would have dismissed it as blasphemy if (1) I were the sort to adopt approaches based on dogma and (2) he hadn’t helped author at least 4 more compilers than I have
Right and I agree! But also in that article states:
Sometimes a “best practice” is not applicable or is inappropriate for a particular organization’s needs. A key strategic talent required when applying best practice to organizations is the ability to balance the unique qualities of an organization with the practices that it has in common with others.
And this is my point.
EDIT: Also in the same document, the “Contextual practice” which what I’m trying to expose.
Surely you need to know what the dependencies are before you call singleton::getinstance()… that point is entirely irrelevant as I need to know what may or may not be available from singleton::getInstance().
With this code:
$foo = singleton::getInstance('foo');
This might break if I pass an invalid string to the method and crash the code at that point. With DI if I type hinted in the constructor, I know that
$foo = $this->foo;
is a valid instance of Foo and I don’t need error handling in my code because I know the code will never get that far unless a valid Foo instance is set.
Incorrect. I said that I don’t use DI in those circumstances where the identity of the dependency is known and will never change because there are no alternatives. In other circumstances, such as with my Controllers and Views, the dependency can be supplied from a number of alternatives, so I use DI in order to specify which alternative to use at that moment in time.
It is when you factor in the change that you have to make to the current object’s constructor, the addition of the $foo property, and the instantiation of the foo object. Even if you are using an autoloader instead of my singleton class you need to factor in the setting up of the autoloader.
I still maintain that my method is quicker and easier t read as it requires only two lines of code in one place instead of multiple lines of code in several places.
The problem with this is that it ignores the fact that you are explicitly breaking encapsulation if you know anything about the internal implementation details. This shows a fundamental misunderstanding of OOP. And no offence but compiler authors understand ASM and low level languages, not OOP. Let’s be clear: When you use DI you are fulfiling a contract. When you inject a dependency that dependency is there to provide an implementation. As the class author, I simply do not need to worry about whether that is a good or unrelable implementation, because it simply doesn’t matter to the class I am using. The person who wrote the calling code is the one making the decision about how their application should work. Otherwise, it’s me, the class author making a decision about how their application should work.
Then you need to factor in the singleton class. Apples to apples it’s this:
class singleton {
public static function getInstance($class) {
reutrn new $class;
}
}
class X {
public function doSomething() {
$foo = singleton::get('foo');
$foo->bar();
}
}
new X;
vs
class X {
private $foo;
public function __construct($foo) {
$this->foo = $foo;
}
public function doSomething() {
$this->foo->bar();
}
}
new X(new Foo);
The singleton approach is longer and it’s longer and longer the more methods you have using the depenency:
class X {
private $foo;
public function __construct($foo) {
$this->foo = $foo;
}
public function doSomething() {
$this->foo->bar();
}
public function somethingElse() {
$this->foo->baz();
}
}
new X(new Foo);
or
class singleton {
public static function getInstance($class) {
reutrn new $class;
}
}
class X {
public function doSomething() {
$foo = singleton::get('foo');
$foo->bar();
}
public function doSomething() {
$foo = singleton::get('foo');
$foo->baz();
}
}
new X;
Each of my Model classes is responsible for a single database table, but sometimes I need to obtain data which is spread across several tables. Sometimes I can achieve this with a JOIN to another table, but sometimes the query is so complicated that I call a method on the other table’s object. This is where my example of my Person object calling the Address object comes into play. All person details and postal addresses are stored on separate tables, and each person can have several addresses for different purposes. In this example I can only use the Address object to obtain a postal address, so I don’t need the ability to inject an alternative object for the simple reason that there are no alternatives. So why should I complicate my code to provide the ability to do something that I know I will never do? wouldn’t this be a violation of YAGNI?
@TomB, beware of this comment. If I need to make a “Payment”, should I let the unknown “client” pass a “Withdrawal” object ? What if that class only deposits and never withdraw ? Honestly, I haven’t really though about it, but it seems dangerous, but scalable, if I let the injection go but limited, but secure, if I make the encapsulation. What would you choose, if any of those two, and why ?
I’ve just read that comment from the Eric Lippert and didn’t really though about it but, I won’t make assumptions that he does not know “high level” languages, that would be rude on my part (he is part of the C# design team, btw - https://social.msdn.microsoft.com/Profile/eric%20lippert/activity )
Also, just to get your complete point of view, in the case of the @tony_marston404 initial sample, where did you construct the “Address” that would be injected into the Person? In the business layer, as a factory, and why?
No. You simply instantiate the specified class without caring about any dependencies which it may or may not have. You don’t need to instantiate and inject any of those dependencies for the simple reason that each dependency is instantiated just before the service from that object is consumed. This is called lazy loading. What DI uses is called eager loading.
If you supply a class name which does not exist then it will crash the system, as it is supposed to do.
But if the ‘foo’ class does not exist then your code will fail when it tries to inject an instance of ‘foo’. Same error, different places.
OOP has nothing to do with Design By Contract, or any other type of contract. It is about classes, methods, properties, instances, inheritance and polymorphism.
So the implementation is no longer hidden? That’s where you break the idea of implementation hiding which goes with encapsulation.
Excuse me, but multiple calls to the same method is not breaking the DRY principle. If I were to duplicate the contents of that method instead of calling it, then THAT would violate DRY.
@tony_marston404 Don’t explain to me… explain to yourself and your colleagues. But the less you need to explain the better, because that is a symptom of high quality application.
In your example, you can ask yourself, should I use the segregation principle? Better yet, if this entity is “fetched in the data layer” and if passed it to the business and than to the UI layer, does it make sense as it is? Image a designer of a twig seeing the signature of this class as ask itself “which of the 200’s methods should I use to display the Person’s name”? Also, does this class, as it is, travels through all the layers?
What I believe that happens is that you’re mixing the entity definition (Person), database access, state of the object, caching, etc… etc… in my eyes, and in the eyes of @TomB (he already mention this), this doesn’t seem good, again… to many responsabilities, and I already saw millions of other’s people code and had to maintain it. And like I said, if you could convince me that this 2000 line of file is feasible “I would hire you”.
I’m out of context of your application and I would prefer to discuss the main topic instead. Otherwise, I won’t be apart of this thread.
But this is the same in DI. My class is passed a fully constructed object, I don’t care what dependencies that class may or may not have…
Exactly my point.
This always crashes if there is a problem:
new Foo(new Bar);
This does not:
class Foo {
public function doSomething() {
if ($x) {
$foo = singleton::getInstance('bar');
}
}
}
It will only crash when the object is in a very specific state. As such, it’s less reliable because it’s unpredictable… it may break it may not, maybe depending on user input or maybe in some unusual circumstance.
You are repeating yourself when you don’t need to. It violates DRY unquestionably. Ok it’s “only one line” but it’s one line multiplied by the number of methods that use the dependency.
No… you’ve been saying DI breaks encapsulation. The part I’m quoting said that “Someone else’s class needs to know about the implementation of yours” which is untrue and if it were true it would only be true because you broke encapsulation somewhere.
But that’s rather a backwards view. If I have a payment class. that class is instantiated somewhere. Whoever instantiated that class is developing the application so will do what they want with it. DI or not is irrelevant here… or I’m not understanding. Can you post a code example that illustrates the problem (ideally two examples, one with DI and one without)
but as I said, having occasional situations where you do not apply best practices, is different to actively avoiding them. Where possible they should be applied, they exist for a reason,
@TomB, hopefully this “simple” system can demonstrate somehow the dependency injection “complexity”. Lets say I’ve got my Bank module, but I opened it up to someone else to use it.
My module:
namespace BankModule;
class BankManager {
private $w;
private $bankComission;
public __construct(AccountManager $w) {
$this->w = $w;
$this->bankComission = 30;
}
public function Transfer(Account $a, Account $b, $amount) {
//do the actual transfer
$this->w->makeDebitTransfer($a, $b, ammount);
//get my comission
$this->w->debitComission($a, bankComission);
}
}
and the outsourced module (that doesn’t even have classes, plain old functions) just uses my module
$bankManager = new BankModule\BankManager(new BankModule\AccountManager());
//fetch bills
foreach($bills as $bill) {
var success = $this->bankManager->transfer($myAccount, $bill.Account, $bill.Amount);
$bill.setPaid($success);
}
Certainly if I give him the code is as secure as it can get but… do I want him to implement the AccountManager or should this be my concern? Do I want him to implement its own transfer where it only deducts half X and deposits Y ?
Also a question you didn’t answer me before, even if it uses OO programing, should you do this:
class ShoppingCart {
private $bankManager;
private $myAccount;
function __constructor() {
$this->bankManager = BankManager(new AccountManager());
$this->myAccount = ...;
}
function payBills() { ... }
}
var $s = new ShoppingCart();
$s->payBills();
or this:
class ShoppingCart {
private $bankManager;
private $myAccount;
function __constructor(BankManager $bankManager) {
$this->bankManager = $bankManager;
$this->myAccount = ...;
}
(...)
}
var $s = new ShoppingCart(BankManager(new AccountManager()));
$s->payBills();
remember that this is main entry point of the application. So where do you resolve all your dependencies?
Right at the begging or do you let each module define it’s own dependencies?
Exactly, so we both agree. I understood, sorry if I did it wrong, from your part that you’re trying to transmit that you must apply, not that you should apply. (obligation vs indication)
