Blog Archive

Thursday, 30 March 2017

Is Best Practice Actually Poor Practice? Dependency Injection, Type Hinting, and Unit Tests...

I've recently been in discussion with a colleague who thinks that dependency injection (DI) is over-used and, in cases where the dependency is a concrete class, unnecessary (in the latter case, he advocates simply creating new objects on the fly). I raised the point that dependency injection allows you to pass in a mock object while unit testing, but he dismissed that as irrelevant and not a valid argument as to why dependency injection is useful. My colleague also stated that type hinting dependencies results in tight coupling, and he would prefer it if we simply abandoned type hints, allowing the developer to pass in whatever they like.

In my opinion, this line of thinking is misguided, but he sent through some links to pages that he felt supported his point of view (including Tony Marston's rant on DI, and the Laravel documentation about 'facades' - which are actually used as an alternative syntax for the service locator [anti-]pattern). I genuinely wanted to understand the reasoning behind his point of view, as it flies in the face of just about everything I have ever read regarding best practice in PHP development. After reading those resources he sent though, I began to notice some misconceptions about what unit testing actually is, as well as confusion about the difference between code that is "strongly typed" (usually good) and "tightly coupled" (usually bad), and also a tendency to blame the wrong thing when problems arise.

Not All Automated Tests Are Unit Tests


There are different types of automated test that can be performed, and sometimes these different types get erroneously conflated. A unit test is used to verify that a particular class behaves correctly in isolation. When you write a unit test, you are only testing that one class - not any of its dependencies (they have their own tests).

That's not to say you can't write a test that covers a number of classes at once, but if you do, it is not a unit test - it is either an integration test (if it deals with the integration between classes), or a functional test (if it tests an entire function regardless of how many classes are involved). Even if you use a unit testing framework (like PHPUnit) to write the test, it is only a unit test if it tests a single unit.

Using Mocks to Test in Isolation


The reason we try to test units in isolation is that we want the test to live with the class, not the implementation of its dependencies. The implementation can change by passing in different dependencies, but we don't care about that, as those dependencies will have their own tests - we only care that this particular class is behaving correctly. If our test fails, it should be because there is something wrong with that particular class (or with the test itself), not because something changed in a dependency. This helps with debugging and provides reassurance that our classes are behaving correctly regardless of external implementation (integration and functional tests are useful for different reasons).

When writing a unit test, it is common to inject mock objects instead of real objects for the dependencies. This has a number of advantages, for example, it allows you to run tests without activating things you don't want (sending e-mails, making database connections, API calls, etc.), it allows you to force the test down paths that would otherwise be difficult to reach, and it allows you to isolate the class you are testing. That's not to say that all dependencies are always mocked. It might be that a concrete class provides what the test needs, and does no harm, so it might be quicker and easier to use that (in which case it is still a unit test, as the dependency is not part of the test subject).

If you create dependencies on the fly though, you no longer have an option - it is not possible to supply a mock object, nor to extend that dependency to tweak the implementation without changing the class that uses it. This is the very definition of tight coupling - it doesn't get any tighter than that. It is no longer possible to unit test that class without also taking the concrete dependency along for the ride.

What, Never Ever Create Objects on the Fly?


There are occasions when it is perfectly acceptable to create objects on the fly, but we need to fully understand the consequences of doing so, and be able to justify it. I find it a good rule of thumb is that the 'new' keyword should only be used in dependency injection containers and factory classes, but (as with all best practice principles) that is just a rule of thumb, not a law. Some occasions where it might be harmless to create objects are:
  • Where your entities form a hierarchical structure, and a parent entity needs to create and initialise a child entity for one of its properties.
  • Where a new value object is needed - a value object represents a value with no identity separate from its properties, and is essentially a data structure (albeit possibly with methods). In some cases I might justify creating a value object in the same way as creating a new PHP DateTimeImmutable object (which is itself a value object).
  • When using a helper class which acts as an extension to the language (for example a string or maths helper class that provides general convenience functions not provided by PHP).
Whenever we do one of these things though, we must bear in mind that the code is now tightly coupled to that class, and be willing to live with the consequences of that. If we ever want to re-use that code in a different situation, will the class that object is based on still be available? Is it possible we would ever want to swap it out for something else? Would we ever want to mock it for unit testing purposes?

What About Those Laravel Facades?


In Laravel, you can make a static call to what they refer to as a facade (not a true facade, more of a proxy), which calls a service locator to find the requested dependency. As long as the service locator is properly implemented, it is possible to populate it with mock objects that can be used in unit testing. This is certainly better than just creating new objects on the fly, as the dependencies are not tightly coupled and can be swapped out relatively easily.

The service locator does still need to be populated though, so there is not a great deal of benefit in terms of setup when using a service locator - populate a service locator, or populate a dependency injection container; it is essentially the same thing, the difference is in how you use it.

Actually consuming a service locator is arguably easier than dependency injection though, because you don't have to declare your dependencies with constructor or setter parameters. All you need is the service locator (and in the case of Laravel, not even that - you can just make static calls wherever you like). That ease of use is what makes it so alluring to some. There are dangers here though...

Hidden Dependencies and Other Dangers


A service locator, whether used explicitly, or via a pseudo facade, hides dependencies from view. This alone has earned it a reputation as an anti-pattern, and a poor choice for enterprise web development. With dependency injection, you can tell from the constructor signature what the dependencies are, and with type hints, you can ensure that only valid values are accepted. With a service locator, you have to examine every line of code in the class, or consult some documentation (and hope that it is up-to-date and accurate) to work out what the class needs in order to run. To my mind, that is a lot more painful than just being explicit up-front with DI.

In addition, code that proxies to a service locator is not very portable. If you use Laravel facades to make static calls to a service locator, you cannot re-use your class in any non-Laravel project without stripping them all out first. Whilst some tight coupling to your framework might be difficult to avoid, there is no need to tie yourself down with facades when dependency injection provides a virtually painless alternative.

Strongly Typed is not Tightly Coupled


In PHP 4, it was not possible to type hint arguments in functions - every argument was 'mixed' and you could pass in anything you like. With PHP 5 came the ability to type hint on a class name, interface, array, or callable, which allowed developers to specify what type of data they expected. In PHP 7, this is taken even further, and we can type hint with scalar types (either strictly, or coercively), and even type hint return values. If, as my colleague suggests, strongly typed arguments lead to tight coupling, why this seemingly unstoppable march toward stronger typing?

It could be argued that it is just fashionable, and that there can be drawbacks to strong typing as it is implemented in PHP (unlike statically typed languages, if you override a method, you cannot change the signature - unless it is the constructor). However, there are a few reasons why I think type-hinted code is much easier to work with. Type hints are self-documenting, giving you lots of useful information on what the argument is for and how it is being used. I like the fact that my IDE can offer code completion and navigation based on type hints (docblocks are OK, but they can get out of sync as development progresses - type hints are always up-to-date). I like the fact that I can catch errors early if the wrong type of argument is passed in, an argument is accidentally missed, or sent in the wrong order - rather than accepting anything only to have the code fail in mysterious and difficult-to-debug ways later on, or have to write my own instanceof checks.

None of that causes tight coupling. Type hinting on concrete classes is more tightly coupled than type hinting on base classes or interfaces, but even with a concrete class (it is not always possible or even desirable to avoid them) it is still possible to extend and mock, so type-hinted dependency injection is always going to be more loosely coupled than having your classes create their own dependencies on the fly.

Another Reason?


Aside from possible misunderstandings like those mentioned above, it appears to me that often, developers who want more 'flexibility' than commonly accepted best practice allows, are actually struggling to understand how to implement best practice. A poor design will result in difficulties implementing best practice, and it is tempting to blame SOLID (or other principles) as being wrong or inappropriate rather than to recognise flaws in one's own design. In my experience, learning OOP is a slow and sometimes painful process. It is not easy to see how and why SOLID principles are necessary. Things like dependency injection, unit testing, using interfaces, and design patterns, are confusing and can appear to be unnecessary or irrelevant. Why go to all that trouble when it works just fine if I do it the way I know?

Exacerbating things, we have popular projects and frameworks (such as Laravel) which are engineered primarily with rapid application development in mind, being used in potentially inappropriate ways for enterprise development. Following SOLID and other best practice principles is less important when your focus is on creating software with a short lifecycle (that may be discarded in a few months' time). Active record (breaking the single responsibility principle), auto-wired dependencies (relying on magic and hiding what is going on), static methods (tightly coupled and hiding dependencies) make development easier, but the resulting software harder to maintain. The phenomenal success of Laravel has much more to do with the fact that it is easy to understand and quick to get started than the use of best practice (that's not to say that you can't use best practice with Laravel if you are disciplined enough).

I think you cannot really appreciate the benefits of some of these things until you get into the habit of unit testing. As the pieces gradually start coming together, SOLID principles become more natural, your code becomes more robust and the whole process of development becomes more of a pleasure. The learning process never stops though.

My conclusion is that SOLID is an invaluable guide to doing things in a way that leads to cleaner, more maintainable, more extendable, more testable code. If I feel tempted to violate any of those principles (or discover that I already have done so without realising it), I need to double check that my approach is not flawed and be certain I understand the consequences.

Naturally, some people will always disagree!

Thursday, 26 February 2015

Domain example.com has exceeded the max defers and failures per hour

When sending e-mail from a cPanel hosting account, you might find that you get an error message similar to this: "Domain yourdomain.com has exceeded the max defers and failures per hour (5/5 (100%)) allowed. Message discarded."

If you are getting that error, it means that a large number of e-mails that have been sent from your domain have been rejected by the receiving mail server. Just increasing the limit is not a good idea, as it exposes the server to blacklisting if a spammer compromises an account. You therefore need to find out why so many sent messages are failing. Here are some suggestions on what to look for:

  1. Do you have a mailing list, perhaps for a newsletter? If so, your list might need cleaning up - too many dead e-mail addresses will push you over the limit. Better still, use a third party mailing service such as MailChimp for your newsletters, so you don't risk getting your own server blacklisted.
  2. Has your account been hacked? Try running an exploit scan (eg. using Configserver Exploit Scanner [CXS]) and make sure all the scripts running on your hosting account are up-to-date.
  3. Are any of your mailboxes forwarding to another mail service such as gmail or hotmail? This is a terrible idea! Delete those forwarders! If you want to get your mail into gmail, you need to 'pull' it from the gmail end, not 'push' it from the cPanel server (likewise for other mail services). Why? Well, if spammers send mail to your mailbox, and your mailbox forwards it to gmail, gmail will reject it (with a message like this: "Our system has detected an unusual rate of unsolicited mail originating from your IP address. To protect our users from spam, mail sent from your IP address has been temporarily rate limited. Please visit http://www.google.com/mail/help/bulk_mail.html to review our Bulk Email Senders Guidelines."). This will count against your failed send limit, causing the above 'max defers/failures' error. It will also get your server's IP address blacklisted, which could cause problems for anyone on the same server sending mail to gmail.
  4. Have any of your mailboxes been hacked? If someone has discovered or guessed your mailbox password, they could be connecting and sending spam. Change your mailbox passwords and run a virus/malware scan on any device that you use to connect to the server.
  5. Login to cPanel and click on the 'Email Trace' icon. Don't enter an e-mail address, just click the 'Run Report' button (this will show details for all mail both in and out of your account). Look through the list for any groups of messages that failed to send from one of your mailboxes - that might give you a clue as to where the failures are coming from.