Hacker News Reader

Comment by orf | Hacker News Reader

> Which changes Email::send signature, which leads to breaking API changes and extensive manual refactoring of the entire codebase.

Only if you decide to change the signature, there are other ways to achieve the same result.

View on HN

Exactly! This is why Lisp has dynamic variables:

    (let ((*emailer* mailgun-client))
      (send email))

Or, using Go contexts (which are just a way of hacking in dynamic variables):

    ctx = email.NewContext(ctx, mailgunClient)
    email.Send(ctx)

And in email.Send:

    emailer, ok := EmailerFromContext(ctx)
    if !ok {
      emailer = defaultSMTPEmailer
    }

For example?

"GetMailPolicySingleton()->SetSendingMechanism(Email::SMTP)" ;)

Sometimes you're sending via SMTP, sometimes via MailGun. If you don't remember to call GetMailPolicySingleton()->SetSendingMechanism() before every email.send() you end up using whatever policy was set last.

> Singleton

What if two threads need to send mail via SMTP and Mailgun...

Singletons have nothing to do with reentrancy or lack thereof.

A 'sending mechanism' property on the email object that defaults to SMTP?

This. There may be other reasons to dislike the email.send() formulation, but it seems like the need to support other mail APIs could be handled relatively easily within this type of OO approach through the use of default values.

I suppose you could add more methods, e.g sendEmailViaMailGun. But that results in the invoker having to have if-else logic (itself a code smell) for selecting which method to call, which is bad for SRP. You could try to fix that by encapsulating that logic into an EmailRouterStrategy class, but that's just another dependency you have to inject, which is not ideal according to TFA.

Couldn't you just add a new SendMethod optional parameter defaulted to the method that was used prior to the refactoring? No breaking API changes, no extensive manual refactoring of the entire codebase, as far as I can tell. This is how I always did refactoring for years, it's immediately obvious to support developers wtf is going on, and there's no requirement to have any "complicated" architecture on every single object in your entire application. Instead, the code largely resembles the simplicity of the real world being modeled.

Is this approach is so dangerous (as we're constantly told) why has it never caused me any problems? Maybe I'm missing something but I've never read an article that explains what it is I'm missing in a way that clicks.

> Couldn't you just add a new SendMethod optional parameter

Adding an optional parameter to an already existing method is not always a backwards-compatible change. For example, in C# this will effectively change method signature and break API.

Holy cow it seems it seems you're right:

http://haacked.com/archive/2010/08/10/versioning-issues-with...

http://haacked.com/archive/2010/08/12/more-optional-versioni...

...yet I've used this technique for years in VB.Net without issue....weird.

Reply on news.ycombinator.com