[RFC] Standard email sending interface

[Petah]

I propose a standard email sending interface. I has always bothered me that every framework/ecommerce platform handles email differently.

Consider this a draft, I will come back later and flesh it out some more.

interface EmailInterface {
    public function addTo(string $emailAddress, string $name = null);
    public function addFrom(string $emailAddress, string $name = null);
    public function addCc(string $emailAddress, string $name = null);
    public function addBcc(string $emailAddress, string $name = null);
    public function addReplyTo(string $emailAddress, string $name = null);
    public function setSender(string $emailAddress, string $name = null);
    public function addHeader(string $key, string $value);
    public function setSubject(string $subject);
    public function addAttachment($file);
    public function setText(string $text);
    public function setHtml(string $html);
}

interface EmailSenderInterface {
    public function send(EmailInterface $email);
}

Things to also consider is adding remove* methods, and other features like priority, inline attachments, return path etc.

[shadowhand]

I would not be in favor of any interface that wasn't immutable. Often it makes sense to have a single email sent to multiple people, in which case an immutable $email makes far more sense:

$email = new Email();
// ... set reply-to, etc
foreach ($users as $user) {
    $sender->send($email->withTo($user->email, $user->name));
}

[samdark]

Additional things to consider:

• Charset.
• Inline attchments i.e. embed($file). This one should return CID which could be used in content afterwards.

[Petah]

@shadowhand how exactly are you expecting it to be immutable? You say "set reply-to" but how are we actually setting it?

Also people can always do $sender->send((clone $email)->addTo(...))

[shadowhand]

@Petah using withReplyTo(...) etc. PSR-7 style, where all with* methods return a clone.

[odan]

I think "immutable" is a implementation detail. A standard (interface) should not make such a decision. A developer should decide if he needs a cloned object.

$objectB = clone $objectA;

[shadowhand]

It's definitely not an implementation detail. It influences both method naming and documentation. See PSR-7.

[odan]

@shadowhand If you say that all objects has to be cloned, you force a very determined implementation.

[mathroc]

@odan an interface tells what is expected of the implementation, if the interface says "$email->setSender('john@snow.north') makes john@snow.north the sender of $email" an implementation cannot make it immutable. and, OTOH, if the interface says "$email->withSender('john@snow.north') returns an new email similar to $email but with sender john@snow.north" an interface cannot choose to instead modify $email and return it. in this case (and almost always I guess) the interface decides if it's going to be mutable or immutable

[odan]

In PHP you can choos beetween the DateTime or DateTimeImmutable class.

http://php.net/manual/en/class.datetime.php http://php.net/manual/en/class.datetimeimmutable.php

The class DateTimeImmutable behaves the same as DateTime except it never modifies itself but returns a new object instead.

Both classes implementing the same interface DateTimeInterface

I think we could make it like that. As developer you can choose between a mutable or immutable class, but the Interface ist the same. That's freedom :-)

[OddGreg]

@odan That's a downright fancy point right there! Something to consider. It seems important to avoid obstructing implementation by introducing unnecessary limitations. From one perspective, anyway.

[peter-mw]

i think @odan is right, if the object is immutable should be a decision of the developer, not the PSR

i also dont like to use the clone function (personal preference)

[shadowhand]

That makes no sense at all. PSR-7 clearly states immutability as part of the specification.

On Fri, May 13, 2016, 18:16 Peter Ivanov notifications@github.com wrote:

@odan https://github.com/odan is right, if the object is immutable or not should be a decision of the developer, not the PSR

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/php-cds/php-cds/issues/15#issuecomment-219181473

Woody Gilk http://about.me/shadowhand

[mathroc]

@odan take a look at the DateTimeInterface You'll see that only read-only method are present, set* & modify are not present in the interface. You can read on Derick Rethanss blog (the main DateTimeImmutable author) why it was done like that.

tl;dr: DateTime already existed but it had one design mistake: it was mutable

[OddGreg]

@shadowhand

I scanned through the thread looking for how PSR-7 has anything concrete to do with this RFC -- other than that email and message interfaces are within the same sphere of influence. I'm not questioning your desire to press the point but rather would like a bit of clarification.

[peter-mw]

@shadowhand @mathroc can you point reason why an EmailInterface should be immutable?

Whats wrong with changing values on the same object?

[mathroc]

I don't have strong feelings about this EmailInterface I just reacted because I thought there was a misunderstanding about immutability.

anyway, I can quote wikipedia:

Strings and other concrete objects are typically expressed as immutable objects to improve readability and runtime efficiency in object-oriented programming. Immutable objects are also useful because they are inherently thread-safe. Other benefits are that they are simpler to understand and reason about and offer higher security than mutable objects.

it does not get much deeper into why on wikipedia, but you can read here a (very) opinionated article about immutable objects: http://www.yegor256.com/2014/06/09/objects-should-be-immutable.html

excerpt:

This is an incomplete list of arguments in favor of immutability:

• immutable objects are simpler to construct, test, and use
• truly immutable objects are always thread-safe
• they help to avoid temporal coupling
• their usage is side-effect free (no defensive copies)
• identity mutability problem is avoided
• they always have failure atomicity
• they are much easier to cache
• they prevent NULL references, which are bad

[OddGreg]

Thanks for that information and references, @mathroc. I've done quite a lot of reading on the subject of immutability -- where and where not to employ it. Indeed, there is a near fanaticism about certain situations, some of which cause me to scratch my head and wonder if it isn't some sort of religion. (No offense intended to anyone.)

I employ a great deal of immutability in my projects, and sometimes refactor to implement it in places where immutability isn't strictly necessary. So in some respects, I've been affected by the global discussion.

That said, I don't believe for a moment that immutability is a holy grail, but rather a concept to keep in mind. Thus, I see no reason to codify immutability in an interface unless there is good cause to do so. I'm not convinced that EmailInterface has a need to be immutable -- so I'd rather that it were not codified. Again, as always, I can be swayed with a reason that harmonizes with my thinking.

Edit: PS: I'm not even sure that an interface can assert (outside of documentation) that the object be immutable. It seems that an abstract may serve that purpose better, but that's not relevant to this discussion.

[SignpostMarv]

I'm wondering why there's not a separate interface for email recipient & message body, i.e.:

interface EmailHeadersInterface
{
    public function setHeader(string $key, string $value);
    public function getHeader(string $key);
}

interface EmailRecipientInterface extends EmailHeadersInterface
{
    public function addTo(string $emailAddress, string $name = null);
    public function addCc(string $emailAddress, string $name = null);
    public function addBcc(string $emailAddress, string $name = null);
}

interface EmailBodyInterface extends EmailHeadersInterface
{
    public function setSubject(string $subject);
    public function addAttachment($file);
    public function setText(string $text);
    public function setHtml(string $html);
}

interface EmailMessageInterface extends EmailHeadersInterface
{
    public function addFrom(string $emailAddress, string $name = null);
    public function setSender(string $emailAddress, string $name = null);
    public function addReplyTo(string $emailAddress, string $name = null);
    public function setRecipients(EmailRecipientInterface $recipients);
    public function setBody(EmailBodyInterface $recipients);
}

interface EmailSenderInterface
{
    public function sendEmailMessage(EmailMessageInterface $email);
}

So implementations could be rather specific (i.e. templated email body formatting without needing to bundle an email sender or recipient validator), mixed (build a body & add recipients) or complete (i.e. swiftmailer/phpmailer)

Ideally I'd also like to see emails be optionally serializable so the messages can be built immediately, but queued for later sending with a cronjob- whether or not this requires a specific interface or just Serializiable I'm not too sure.

p.s. in addition to allowing for tasks-specific implementations, separating the interfaces leaves scope for interfaces & implementations designed for retrieving emails from a mailbox or processing them to do tasks that don't send emails (message filtering, reformatting, translation, encryption, debug rendering etc.)

Mailing List use-case

$mailer = new EmailMailer($mailerConfig);
$body = new EmailBody();
$messages = array();
$headers = array();
foreach ($recipientGroups as $recipientGroup) {
    $recipients = new EmailRecipients();
    foreach ($recipientGroup['to'] as $email) {
       $recipients->addTo($email);
    }
    foreach ($recipientGroup['cc'] as $email) {
       $recipients->addCc($email);
    }
    foreach ($recipientGroup['bcc'] as $email) {
       $recipients->addBcc($email);
    }
    $messages[] = new EmailMessage($body, $recipients, $headers);
}

foreach ($messages as $message ) {
    $message->addFrom('foo@example.com', 'foo');
    $message->addReplyTo('bar@example.com', 'bar');
    $mailer->sendEmailMessage($message);
}

Single-recipient use-case

$mailer = new EmailMailer($mailerConfig);
$recipients = new EmailRecipients();
$recipients->addTo('foo@example.com');
$messages = array();
foreach ($bodies as $body) {
   $messages[] = new EmailMessage($body, $recipients, $headers);
}
foreach ($messages as $message) {
    $message->addFrom('foo@example.com', 'foo');
    $message->addReplyTo('bar@example.com', 'bar');
    $mailer->sendEmailMessage($message);
}

Ecommerce Serialization use-case

$someoneBoughtTheseThings; // object for order of products/digital purchases etc.
$messages = array(
    makeEmailForBuyer($someoneBoughtTheseThings),
    makeEmailForSeller($someoneBoughtTheseThings)
);
foreach(makeEmailsForSuppliers($someoneBoughtTheseThings) as $message) {
    $messages[] = $message;
}
foreach ($messages as $message) {
    $db->query(
        'INSERT INTO `messages` (`queuedDateTime`, `message`) VALUES(NOW(), ?)',
        $message->serialize()
    );
}

accompanying cronjob

$mailer; // implementation of EmailSenderInterface
while ($ContinueToSendEmails) {
    $message = $db->getNextEmailInQueue();
    $mailer->sendEmailMessage($message);
    $db->removeFromQueue($message);
}

[mathroc]

Edit: PS: I'm not even sure that an interface can assert (outside of documentation) that the object be immutable. It seems that an abstract may serve that purpose better, but that's not relevant to this discussion.

@OddGreg You're absolutely right, an implementation can be mutable even if the interface intend for it to be immutable. There's nothing in PHP that can enforce that. So yes, only the interface & methods signatures and their documentation can explicitly tell that a class should be immutable (or mutable). But that's part of the method behavior. and nothing can prevent an implementation to do something else. eg: an implementation of \Psr\Logger\LoggerInterface could try to delete files from your server instead of logging stuff

[geggleto]

Might I suggest a name change to the Interface...

EmailInterface ==> EmailMessageInterface

Reason: There are a few different parts of the email workflow. What you are describing here is the message, not say the transport mechanism.

[mathroc]

fwiw maybe we can consider dropping the Interface suffix is not really needed and the namespace is enough to not make it a problem for implementation that can't think of a more specific name :)

[peter-mw]

@mathroc maybe its good for the suffix to stay so you can write

use \Pcs\Email\EmailMessageInterface;

class EmailMessage implements EmailMessageInterface {}

[mathroc]

@peter-mw yeah that a little bit easier if you can't think of another name but I find it pretty annoying on code using said interface, eg:

function doStuffWithAnEmail(EmailMessageInterface $email) { // I don't care that it's an interface
   // ...
}

function doStuffWithAnEmail(EmailMessage $email) { // that's enough information
   // ...
}

some reading: http://verraes.net/2013/09/sensible-interfaces/

[OddGreg]

@mathroc

Actually, that's a fair point, although I should say that I'm not particularly annoyed by the use of Interface or Contract (depending on what opinionated framework or package.) I mean, let's face it -- using Interface or Contract is completely unnecessary. As with other things, these are only conventions, possibly to echo intent rather than as a part of the language.

Older languages such as Pascal used Initialization, Interface, Implementation and Finalization all within the same source file, and even then people added unnecessary directives to the name of classes.

It's a choice made largely by the PHP community to use Interface or Contract when not strictly necessary. Convention has its usefulness, but sometimes it adds nothing but kudos for the dude who declared it as a standard.

I'm irritated more by the philosophical arguments on such subjects, stated in such a manner as if to sound authoritative or true, when it is all, in fact, opinion and choice. (Some use shaming and ridicule to drive their opinions into things. I hate that even more.)

We're programmers not fashion designers. Let's just agree on what we think the larger community is likely to accept. I imagine that we will have to vote on this too, which is a maddening detail. Since the language doesn't require Interface in the identifier (that's what interface right next to it is for) then I'm all for leaving it off.

[castarco]

A small idea: I think it is a good idea to allow immutability and use methods like with*(), but it would be nice to combine these points with a Builder interface to allow having a transient mutable object before "finishing" it. This is a typical good practice from the architecture viewpoint and also from the performance viewpoint since we avoid too many object constructions and destructions.

Something like (the name is not meant to be definitive) EmailBuilder, with methods similar to the with* ones, but in this case "set*()", and one last method: build() which should return an Email interface.

[sagikazarmark]

@shadowhand came up with the perfectly correct point: Email is a Value Object, should be immutable.

Here is the most incorrect question I could find in this thread:

Whats wrong with changing values on the same object?

Simple: it's not the same object.

This is at least the third RFC where people simply don't understand what shared state vs functional approach is.

See my comment in the PSR-7 RFC: https://github.com/php-cds/php-cds/issues/30#issuecomment-222821681

The same applies here: a Value Object should be immutable. The value identifies the object. If you change the value, it's a different object. Since we are literally talking about objects, this means a different reference in our case.

Not to mention the completely inappropriate reasoning about what an interface declares and what not, wrong examples with DateTime, personal preference of "not using the clone function" (which is not a function BTW), insane naming ideas and so on. These things make me quite sad, because dispite all the fights in the FIG, their standards are professional in terms of technical correctness. Looking at this discussion: I see a bit lack of professionalism.

Sorry for this last paragraph, but I have the same feeling in quite a few other discussions as well. Maybe the process is better than FIG's (although this is subjective), but if the quality can't be as good as PSRs (which admitedly have flaws in them, but still quite well-designed standards) then what's the point?

[castarco]

@sagikazarmark For the sake of clarity. I think you are overreacting. I think that "value objects" are THE way, as I've stated in my previous comment.

I only talked about also adding to the "Pcs" some interfaces to allow implementing the "Builder Pattern" because is probable that the implementations of the Email interface will have a lot of properties, and having to construct such objects will force programmers to create constructors with too lengthy parameters lists.

Using the builder pattern we can avoid this problem without having to rely on the "withProperty"-like methods, which will trigger a lot of objects constructions and destructions.

Also for the sake of clarity: the builder objects, which should implement the builder interfaces, MUST NOT implement the Email interface to ensure that we cannot pass mutable objects through the methods & functions that accept such interface.

Regards.

[sagikazarmark]

There was nothing personal in it. Again: ignore if does not apply to you. Maybe I am overreacting, but I saw a serious danger that this proposal goes into the wrong way.

Using the builder pattern we can avoid this problem without having to rely on the "withProperty"-like methods, which will trigger a lot of objects constructions and destructions.

I think this has been discussed in the PSR-7 proposal more than a thousand times. Object cloning is relatively a cheap operation. And if you don't preserve a reference to the "old" objects, they will be garbage collected. End of story.

[castarco]

@sagikazarmark My proposal is not against having the withProperty-like methods in the Email interface, my proposal is against forcing programmers to use them at construction time. Having a separate builder interface does not harm the main proposal idea of having immutable "value objects".

In addition, the performance impact of relying on clone operations is easily measurable and not negligible. Is good to have the garbage collector, but not to abuse it, specially when there are more clean, faster and cheaper solutions.

If you don't work in systems with very high loads is possible that you don't care about this, but there are people having to deal with this type of problems every day. Performance is important.

[sagikazarmark]

@castarco I am not accusing your proposal with anything. I reacted to the overall discussion in this thread.

About cloning: it depends. Although I haven't done any benchmarks myself yet, I didn't have any performance issues in mid-load applications. I remember that the FIG did some performance tests. I would like to see some stats though. Until then, I disagree with you: mutable builders are NOT cleaner, and the performance gain is negligible.

[mathroc]

@sagikazarmark My proposal is not against having the withProperty-like methods in the Email interface, my proposal is against forcing programmers to use them at construction time. Having a separate builder interface does not harm the main proposal idea of having immutable "value objects".

it does make the interface more complex and thus harder/longer to implement and more prone to bugs (both in the interface and in the implementations)

In addition, the performance impact of relying on clone operations is easily measurable and not negligible.

can we see some data ? I'm believe there is a performance impact on using valueobject (but I'm not even sure) and I'm not sure at all the impact is "not negligible"

anyway, the builder interface could also be provided outside of this RFC, either as a standalone library or another RFC

[Petah]

I would like discussion to be more about how best to represent an email and an email sender rather than if immutable is in the best interest. Lets bench the immutable subject for now.

[odan]

Is the PSR-7 really 100% immutable? Well, maybe depending on the implementation it's more or less just a "wrapper" for a PHP stream.

In situations where an actual PHP stream is wrapped, immutability is impossible to enforce, as any code that interacts with the resource can potentially change its state (including cursor position, contents, and more). Source: http://www.php-fig.org/psr/psr-7/

The FIG recommends:

Our recommendation is that implementations use read-only streams for server-side requests and client-side responses. Consumers should be aware of the fact that the stream instance may be mutable, and, as such, could alter the state of the message; when in doubt, create a new stream instance and attach it to a message to enforce state.

So the internal stream could be writable, that means mutable. The PSR-7 interface just "fakes" a functional programming paradigm. Malicious tongues claim that PSR-7 is a hybrid between immutable and mutable. The interface design itself can not prevent what's going on in the implementation. Even the FIG has recognized that. I think we have to be careful if you say this MUST be immutable, but technically it could be mutable nevertheless.

[sagikazarmark]

Yes, the stream interface is kind of an exception, but it is not just a value object. Hence the stream nature, it includes quite a few behavioural functions as well. So it can never be immutable.

But it doesn't make the immutable argument any worse. The fact that a behavioural thing does not allow immutability doesn't mean it just "fakes" the functional approach.

[TorbenKoehn]

@OddGreg

*Interface, *Trait and Abstract* are needed because of overlapping names.

If you have a YourFramework\Mail namespace, you might have

MessageInterface, AbstractMessage and Message in there, maybe even a MessageTrait to build an own AbstractMessageeasily, which directly tell you: One is just an interface, one is an abstract implementation you can extend and one is a specific implementation, one is a trait.

What would you call those? Or would you do Mail\Interfaces\Message, Mail\Abstracts\Message or what exactly? Since you can't have Mail\Message for the interface, the abstract class, the trait and the implementation.

This is not some basic $strTitle, $objUser, $arrFlags hungary-notation-stylie-hipster-decision to look pro, it's a simple way to avoid collision and to directly make clear what you're looking at as soon as you start typing Mes.. in your IDE.

You'd also need to change all existing standards, since they all use *Interface

[sagikazarmark]

Interface, *Trait and Abstract are needed because of overlapping names.

These suffixes/prefixes are just evil. Don't do it. When you typehint for a name, do you await there an interface? Or an implementation of an interface?

Someone linked Mathias Verraes's Sensible interfaces post. That is the rule of God.

Abstract is also evil. It indicates that you have no idea why you need inheritance.

Ideally you have the contract separated from the implementation. For example, you have a standard package with the interfaces and an implementation package. Sounds familiar?

Also, it is becoming more and more common to separate interfaces within a package as well. Like using a contract namespace or something. Namespaces are to separate your code, not weird naming rules.

[TorbenKoehn]

@sagikazarmark I think that's nonsense and I'll explain why (Sidenote, I'd like to hear alternative naming conventions when you have an interface, a trait and an implementation with the same name since it's really, really common to put an interface next to its implementation. Using a Contracts namespace is just as bad as using a Traits, Mixins, Interfaces or Abstracts namespace and just as bad as suffixing (Come on, it's just prefixing in the end))

Abstract classes make sense for traits that require an interface and classes that don't extend anything else (Primarily for type-hinting, but also for consistence)

Imagine the following:

<?php

interface MessageInterface
{
    public function getHeaders();
    public function getBody();
    public function __toString();
}

trait MessageTrait //traits can't implement interfaces
{

    protected $headers = [];

    public function getHeaders()
    {

        return $this->headers;
    }

    //abstract function getBody(), implied through interface, but not when using this trait alone

    public function __toString()
    {
        //IDE error, code-smell (not using the interface and not having getBody() will fail at this point)
        return (string)$this->getBody();
    }
}

Now this is a pretty bad approach, since you're not enforced, in any way, to implement MessageInterface when using MessageTrait. Don't take that I'd implement it this way, it's just to show you where the problem resides. Traits can implement interface-functionality that is rather default, but the trait itself is optional.

You might implement MessageInterface alone and implement everything yourself or you might implement MessageInterface and use MessageTrait to get basic functionality and just need to implement getBody(). This gets around inheritance really good and also works in many good cases (and is also a process I enforce whenever I can). You always just typehint the MessageInterface

Now, inside __toString() you call getBody() and your IDE will directly tell you: It's not there.

Would you now put a check into __toString? (if (!($this instanceof MessageInterface)) throw exception?

The best way to solve this is by using an abstract base class instead of a trait, coupled with the interface

abstract class AbstractMessage implements MessageInterface //abstract classes can implement interfaces
{

    protected $headers = [];

    public function getHeaders()
    {

        return $this->headers;
    }

    public function __toString()
    {
        //No IDE errors, no potential code-smell
        return (string)$this->getBody();
    }
}

Now people can stil decide to implement MessageInterface and implement all methods alone or just extend AbstractMessage and have the basic functionality available with just overwriting specific parts.

I never typehint abstract classes, but they are the best fallback until traits finally can implement interfaces. When they finally can, I'll probably ditch abstract classes completely as well.

Moreover, using your naming, my Message class would look something like this:

<?php

use Contracts\Message as MessageInterface;
use Mixins\Message as MessageTrait;

class Message implements MessageInterface
{
    use MessageTrait;
}

So if you need to alias them when you want to use at least two of them, why not just call them that directly?

I don't want a Contracts namespace, so how would I name my classes when I want them to be in the MyFramework\Mail namespace? As short as possible?

[sagikazarmark]

We have quite a huge disagreement here and from my POV all that you said is simply bad design and naming.

Sidenote, I'd like to hear alternative naming conventions when you have an interface

A quite common way is to have a subnamespace for implementations.

Come on, it's just prefixing in the end

Okay, please start using prefixing in your real life: You drive a Toyota Car, not a Toyota. You eat a Sandwich Meal, not a Sandwich... Lack of logical thinking is what usually kills programming.

The trait example is quite bad. Traits are pretty bad implementation of multiple inheritance. Anyway, if you implement a trait like that in your example and use it without an interface, then please immediately stop programming. It is a HUGE developer error. And it's not even about how you design your code.

You always code against contract. If not, then please goto the above suggestion.

I simply don't want to address all your points because I disagree with 100% of them.

Would you now put a check into __toString? (if (!($this instanceof MessageInterface)) throw exception?

Nonsense. Anyone doing this goto above.

So if you need to alias them when you want to use at least two of them, why not just call them that directly?

No you don't. You can use the FQCN of the interface.

I don't want a Contracts namespace, so how would I name my classes when I want them to be in the MyFramework\Mail namespace? As short as possible?

Too bad, that's what namespaces are for.

[mathroc]

@sagikazarmark I think that's nonsense and I'll explain why (Sidenote, I'd like to hear alternative naming conventions when you have an interface, a trait and an implementation with the same name since it's really, really common to put an interface next to its implementation. Using a Contracts namespace is just as bad as using a Traits, Mixins, Interfaces or Abstracts namespace and just as bad as suffixing (Come on, it's just prefixing in the end))

I agree that having a dedicated namespace is not much better than having a prefix or suffix (yet a little bit better imho).

I don't think prefixes & suffixes matter much for trait and abstract class as you probably won't be typehint on them. however I agree with @sagikazarmark that you should not have them on interfaces

and here is an exemple of you can do that:

interface Message
{
    public function getHeaders();
    public function getBody();
}

class MemoryMessage implements Message {
  private $headers;
  private $body;
  public function __construct(array $headers, $body) {
    $this->headers = $headers;
    $this->body = $body;
  }
  public function getHeaders() {
    return $this->headers;
  }
  public function getBody() {
    return $this->body;
  }
}

class JsonFileMessage implements Message {
  private $path;
  public function __construct($path) {
    $this->path = $path;
  }
  public function getHeaders() {
    return json_decode(file_get_contents($this->path))["headers"];
  }
  public function getBody() {
    return json_decode(file_get_contents($this->path))["body"];
  }
}

class PdoMessage implements Message {
  private $pdo;
  private $id;
  public function __construct($pdo, $id) {
    $this->pdo = $pdo;
    $this->id = $id;
  }
  public function getHeaders() {
    return $this->unserializeHeaders($this->pdo->fetchColumn('select headers from message where id = ?', [$this->id]));
  }
  public function getBody() {
    return $this->pdo->fetchColumn('select body from message where id = ?', [$this->id]);
  }
}

if your implementation cannot have another name that your interface it means that there is only one possible implementation of your interface. why do you need an interface then ??

[sagikazarmark]

This is the less harsh and aggressive interpretation of what I said. :smile:

[TorbenKoehn]

@mathroc So you rather want new MemoryMessage() instead of new Message()? You're basically saying "Suffixing is bad, rather prefix" and you're not even using a namespace for the prefix. I think that's even worse.

@sagikazarmark Tell me exactly one reason why traits would be bad for multiple inheritance. Combined with interfaces they make a really good setup for it.

I look at the larger frameworks, I see ContainerAwareInterface coupled with ContainerAwareTrait and that all available in a ContainerAwareCommand providing it out of the box (If there'd be one requirement that ContainerAwareCommand can't provide by default, it would need to be abstract)

Anyway, if you implement a trait like that in your example and use it without an interface, then please immediately stop programming. It is a HUGE developer error. And it's not even about how you design your code.

Read again and calm down before you rant on me please. I'm programming for over 10 years now and you won't have it easy telling me I am bad at it or I don't get basic design principles. Take a look at my repositories. Think before you say something stupid like stop programming.

I'd never, ever implement a trait that can't be used without the corresponding interface, that's exactly what I told you above. Before I do that, I use an abstract class instead of the trait or separate the common trait logic that doesn't require the interface into another trait and then use an abstract class or a direct implementation.

No you don't. You can use the FQCN of the interface.

You do. I don't need to.

Too bad, that's what namespaces are for.

Namespaces are for separating interfaces from classes? I asked for a solution without separating them. I don't want my interface and my class separated when they are basically the same, one is just the definition and the other the implementation. That's like creating an Implementations namespace where you put all your concrete implementations in, that sounds like bad design for me.

[sagikazarmark]

Tell me exactly one reason why traits would be bad for multiple inheritance. Combined with interfaces they make a really good setup for it.

They are bad exactly because of your examples: it allows bad decisions to be made. Look at other languages for multiple inheritance.

Think before you say something stupid like stop programming.

It wasn't meant for you, it wasn't personal. It was meant for someone who does those stupid mistakes you outlined. Generally, I think you won't make this mistakes, but I disagree with what you say as a reasoning why we should still mess with inheritance and naming for the sake of avoiding programmer errors.

IMO the correct solutions are what @mathroc and I mentioned:

1. Name your implementations. Don't just call them Message. Of course it is not always possible, so:
2. Use namespaces. Mail\Message and Mail\Message\Message or Mail\Message\Email.

Again, think about how you name things in the real life and apply the same logic for your model.

[mathroc]

@mathroc So you rather want new MemoryMessage() instead of new Message()? You're basically saying "Suffixing is bad, rather prefix" and you're not even using a namespace for the prefix. I think that's even worse.

@TorbenKoehn you could call it \Something\Memory\Message, \Something\Message\Memory or \Something\MessageMemory. I wouldn't care. The only thing I'd like to avoid is "Interface" anywhere in the FQCN.

[TorbenKoehn]

@sagikazarmark You don't have to apply everything to real life, PHP is not real life and programming is not real life, it's proven that OOP has nothing in common with real-life relations. In real-life you can have the same word for different meanings.

Then lets take a better example. You're writing two libraries, a logger and a cache. Both provide awareness-interfaces for implementations to react to and attach them on-demand.

I'd implement it like this:

<?php

interface CacheAwareInterface
{

    public function setCache(CacheInterface $cache);
}

trait CacheAwareTrait
{

    protected $cache = null;

    public function setCache(CacheInterface $cache)
    {

        $this->cache = $cache;

        return $this;
    }
}

interface LoggerAwareInterface
{

    public function setLogger(LoggerInterface $logger);
}

trait LoggerAwareTrait
{

    protected $logger = null;

    public function setLogger(LoggerInterface $logger)
    {

        $this->logger = $logger;

        return $this;
    }
}

No need for any abstract classes here, but this is multiple inheritance in PHP at its best.

One can just implement CacheAwareInterface and implement setCache, one can use the trait on any object, even an inherited one, you can just use multiple awareness-interfaces and traits at the same time. Trait-implementations can be swapped out part-by-part.

class CacheableAndLoggableThing implements CacheAwareInterface, LoggerAwareInterface
{
    use CacheAwareTrait, LoggerAwareTrait;
}

You're free to e.g. drop the LoggerAwareTrait to implement your own property and setter. (Sidenote: How would you name those? CacheAware? Both? But that's not a noun, so how do you apply that to your Toyota-example? This is where your real-life stuff fails and for the sake of consistency, should stay failed)

Now lets take an example where you have to go abstract: Imagine your logger requires you to log to a specific channel you select or your cache wants to know a cache-format for the data you store (Depending on what it is, JSON can be faster than Serialize and vice-versa)

interface CacheableInterface extends CacheAwareInterface
{
    public function getCache();
    public function getCacheSaveType();
    //isHit, bla bla bla
    public function cache();
}

interface LoggingInterface extends LoggerAwareInterface
{
    public function getLogger();
    public function getLogChannel();
    public function log($message);
}

trait CacheableTrait
{
    use CacheAwareTrait;

    public function getCache()
    {

        return $this->cache;
    }

    public function cache()
    {

        $this->cache->save($this, $this->getCacheSaveType()); //Smelly, we don't know if $this implements CacheableInterface

        return $this;
    }

    //You could do
    abstract public function getCacheSaveType(); //which is pretty redundant API-wise and makes refactoring more annoying

    //You could also give a default impl.
    public function getCacheSaveType()
    {

        return '???'; //but with what? Throw an exception? What do you know what cache-types exist and what the implementor needs?
    }
}

trait LoggingTrait
{
    use LoggerAwareTrait;

    public function getLogger()
    {

        return $this->logger;
    }

    public function log($message)
    {

        $this->logger->logToChannel($message, $this->getLogChannel()); //Smelly, we don't know if $this implements LoggingInterface

        return $this;
    }

    //See above for the obvious same reasons
}

//Solution
trait CacheableTrait
{
    use CacheAwareTrait;

    public function getCache()
    {

        return $this->cache;
    }
}

abstract class AbstractCacheable implements CacheableInterface
{
    use CacheableTrait;

    public function cache()
    {

        $this->cache->save($this->getCacheSaveType()); //It's all good, since we _know_ we implement CacheableInterface

        return $this;
    }

    //extending still requires implementation of getCacheSaveType()
}

trait LoggableTrait
{
    use LoggerAwareTrait;

    public function getLogger()
    {

        return $this->logger;
    }
}

abstract class AbstractLoggingObject implements LoggingInterface
{
    use LoggingTrait;

    public function log($message)
    {

        $this->logger->logToChannel($message, $this->getLogChannel()); //All good

        return $this;
    }

    //extending still requires implementation of getLogChannel()
}

Try to ignore syntax-errors please, it's about the semantics.

What you're proposing is to ditch the abstract approach in this case and have each implementor define cache()/log($message) for themself, which is basically the same implementation on all of them so you ditch DRY in favor of someone said abstract is evil.

Abstract isn't really a good thing, I never said that. But it's the best thing you can use in such situations until traits can't implement interfaces. And traits are pretty awesome for multiple-inheritance. You just can't typehint them. All three constructs in PHP (Interfaces, Abstracts, Traits) have their advantages and disadvantages. You can only get a really stable and clean code-base if you use the best of all 3 of them. Not avoiding one over the other.

Again, stop thinking about naming programming things after real-life things. We don't want to make PHP green, aesthetic and gorgeous, we want to write efficient, lightweight and clean software.

[sagikazarmark]

No need for any abstract classes here, but this is multiple inheritance in PHP at its best.

I think I wasn't clear. I don't say one should not use traits. I say the way multiple inheritance is implemented in PHP is not so good, exactly because it allows the badly developed code you mentioned.

This is where your real-life stuff fails and for the sake of consistency, should stay failed

Well, I don't think the same rules can be applied everywhere. Of course programming is not real life, but your domain is usually modelled after real life things.

In this cases I prefer having names like HasCache or AcceptsCache. Again, someone linked here the Sensible interfaces post. That answers many of your questions.

    //You could do
    abstract public function getCacheSaveType(); //which is pretty redundant API-wise and makes refactoring more annoying

This is the way to go. It might be redundant, but since your trait cannot implement an interface, and you do require that method, you have to require an implementation for it.

I don't say you should avoid using them. I say create model which is logical. Use abstract, when inheritance is necessary, use trait when not, and always use interfaces. But all the three has their roles and as such, can be named accordingly. the real life example is something that should make this easier to understand, but if does not for you, then find your own example. The point is that you are working with a model and not object types.

[TorbenKoehn]

@sagikazarmark I get your points, I just don't see it the same way. I dislike the hungary-notation and I dislike stuff like ISomething (Always sounds like Apple made them), but my (very intensive) experience with PHP lead me to using suffixes/prefixes instead of trying to have everything named real-life like. I try to have wording sense in new Xxx() and the stuff I build around it always follows the same pattern, XxxInterface, XxxTrait and AbstractXxx (Honestly, I don't like AbstractXxx, I personally used XxxBase before but I stuck to AbstractXxx just to stick to the more common conventions)

You're optimizing for Typehints, which is your thing. In turn you do (@mathroc) new Memory() (Meaning Xxx\Message\Memory) or new MessageMemory() (Which, again, points at Memory, not at Message) or you need a shitload of aliases. I don't.

It's easy as that.

[sagikazarmark]

ISomething (Always sounds like Apple made them)

No, Microsoft did :stuck_out_tongue_closed_eyes:

Forget the real life thing. I usually tell this to people as an example because there are logical example in the real life (like the car example). But of course it doesn't mean you have to name things after real life. But when you think about your application's model, you want well-named entities. Modern programming paradigms all push that (DDD). Also, programming has changed: business is more and more involved into the development, which inherently means that they need to understand what you are talking about. You need to be able to model the business processes in a way that your client understands it. And in business processes, there are no abstract classes. If you keep this in mind during software design, you won't need to do messy things in your code.

[TorbenKoehn]

Well, basically you can say, it doesn't matter at all what style you take, the important thing is that you're consistent with it. I know it too well that adapting other standards is a hard task to do, since you have your own view of things and mostly have good reasons for it.

Let's keep it at that and accept, that both styles have their meaning and purpose for different reasons.

What's more important is the fact that all other PSR interfaces are named with *Interface and it's just another standard that has been adopted, not a wrong one.

So now that it's like that (and because of the amount of already existing libraries), it should stay like that. And here again: Let's stay consistent. Over the whole PSRs.

Nothing keeps you from doing

<?php

namespace YourFramework\Http\Contracts;

interface Message extends \Psr\Http\MessageInterface
{
}

and you got your preference back :)

[sagikazarmark]

I agree, consistency is important, but I don't think that these things cannot be changed. If you take a look at FIG conversations, you can see that more and more voice says that it was a huge mistake in the first place.

[TorbenKoehn]

These things can't be changed, because changing it will start a shitstorm from the other side. Everyone would need to re-implement. Everyone would need to update everything. It would be a huge update-mess.

Of course you can work with versioning, but then you have "Middleware before 2.0" and "Middleware after 2.0", which completely breaks the whole sense of having interop-middleware at all.