[Suggestion] Remove the Interface suffix in interface names

[broadway]

[Suggestion] Remove the Interface suffix in interface names

[henrikbjorn]

👍 makes a lot of sense.

[asm89]

👍, let's try to do this sometime soon. Again a PR would be great. Could be combined with #2 maybe.

[sstok]

👍

[simensen]

It is possible I could do this if it isn't already being taken care of by someone else.

[kelvinj]

hey Beau @simensen have you started this?

[simensen]

@kelvinj I have not. :) I wanted to make sure someone else wasn't already working on it. :) Will probably be a considerable amount of work (or at least a considerable amount of renames and whatnot) so I didn't want to double-up on that effort.

[kelvinj]

@simensen Hmmm… might put phpstorm to work and see what the upshot is…

[simensen]

@kelvinj Have at it. :)

[mbadolato]

One thing to keep in mind in terms of BC is that these changes will require renaming some existing (non-Interface) classes. For instance, there is currently a CommandSerializer class which will will need to be renamed so that CommandSerializerInterface can drop the suffix.

While it isn't a huge deal to rename the conflicted classes (especially using PHPStorm), it does create a problem if any projects are using the classes directly. The abstract CommandHandler comes to mind first and foremost. Anyone extending that class will have a problem. Unless, of course, CommandHandlerInterface gets renamed to something other than CommandHandler

[kelvinj]

You could easily say the same about interfaces themselves.

The approach I'm taking is to rename the interface to represent what it does, not what it is. For example, CommandHandlerInterface becomes HandlesCommands, which is more in keeping with the role of interfaces.

This does mean that the names of the default implementations can remain as they are. Separately, I was intending to raise the issue of some abstract classes being preceded with 'Abstract' while others are not. Not a huge deal but would be good to have a general rule of thumb.

I'll try to submit a PR either later or tomorrow am, and then people can tear it apart as they wish.

[mbadolato]

Yep, either way works and again this isn't a huge deal. Proper naming will alleviate some of the issues. Just wanted to mention it for posterity :)

[stof]

This does mean that the names of the default implementations can remain as they are

Well, these default implementations should also be named according to what makes them special. There is no reason to have a priviledged name for them.

[kelvinj]

@stof I agree entirely with your stance when applied to concepts that naturally have multiple implementations.

[kelvinj]

Having started #23 I think the one question that remains open to me is… is this kind of change important enough right now?
Broadway has a lot going for it, namely, it's really well thought through and it's in production, so really, does it matter if the interfaces have an Interface suffix at this point in time?

Qandidate are likely the only ones with production apps that rely on it, and it's their baby, but then it's also relevant to anyone with an interest in contributing.

For me, even though I find interfaces with this naming easier to understand as I navigate the code, it isn't a show stopper.

I'm more interested right now, for example, working on different types of projection backends.

[wjzijderveld]

It might be better if issue #2 is resolved before removing the suffix. That issue might cause other naming collisions.

After that we should look again at the namespace suffix, renaming interfaces to have more sensible names might even be step 3?

[kelvinj]

@wjzijderveld so for example, changing AbstractEventDispatcher to an EventDispatcherInterface interface would be step 1, right?

What are you advocating to be step 2?

[kelvinj]

Namespace suffix sounds like Broadway\EventDispatcher\Interface\EventDispatcher

[wjzijderveld]

Yes, that would be step 1.

With step 2 I meant this issue (#12), so only drop the Interface suffix and rename the Interface if there are collisions.
In step 3 we can take a look at Interface names in general.

Splitting it would also split the discussions about naming, making it a bit more manageable.

[kelvinj]

ok cool, I'm all for making stuff more manageable.

[stof]

Broadway has a lot going for it, namely, it's really well thought through and it's in production, so really, does it matter if the interfaces have an Interface suffix at this point in time?

such change can only be done now: it is a BC rbeak, so doing it later in time will become impossible (until a next BC-breaking major version).
the fact that only Qandidate runs Broadway in production right now means they would be the only ones affected by a change done now, while it will be a pain to do it later

[simensen]

the fact that only Qandidate runs Broadway in production right now means they would be the only ones affected by a change done now, while it will be a pain to do it later

👍

Indeed, I'm currently in the process of trying to get at least one project up and running on Broadway. It will be a pain to make these types of changes a month or two from now.

That said, BC breaks are going to happen eventually and we can leverage semver for that to a certain extent. We can target renaming the interfaces for the next major release (or minor release since we're on 0.x or whatever).

Still, doing it soonish would probably still be my vote. I'm happy to help however I can.

[stof]

@simensen such renaming are hard BC breaks (lots of changes to do everywhere), so it is much easier to do it soonish. This is why I opened the suggestion as soon as it has been open-sourced

[kelvinj]

@stof @asm89 @simensen @henrikbjorn @wjzijderveld @mbadolato

Opinions please, on how would you like to see the naming changed around an EventSourcedEntity, for example.

	Approach	Class	Interface
0	Current	EventSourcedEntity	EventSourcedEntityInterface
1	Infrastructural like PSR	AbstractEventSourcedEntity	EventSourcedEntityInterface
2	Behavioural interface	EventSourcedEntity	SourcesFromHistoryOfEvents
3	Interface as a type	AbstractEventSourcedEntity	EventSourcedEntity
4	More descriptive class	ConventionBasedEventSourcedEntity	EventSourcedEntity
5	Something else…	?	?

On reflection, for me, it's either 1 or 4… probably erring towards 4.

I think it would be well worth writing something similar to the PSR coding conventions to clarify for other contributors.

[henrikbjorn]

I would have 2 but if that isnt possible then 4. I really like interfaces that are natural to read.

[simensen]

My order of preference would be: 4, 2, 1.

As I understand it, 1 is pretty close to 0 / current, the only difference being that any abstract classes that are not already named Abstract* would be renamed? I think that would be helpful and more consistent so even that would be a bump in the right direction.

[stof]

I would vote for 4.

The option 3 does not only half of the work IMO. Abstract does not describe what makes it special, and you are still reserving a privileged name (through less privileged than EventSourcedEntity) for a particular implementation (you can have several abstract implementation based on different behavior)

Note that I would not enforce interface names to be nouns through, as it might depend on the use case. SerializableInterface and TransferableInterface (which is a bad name as the repository is able to transfer other objects, not itself) looks good candidates for a different name, either based on an adjective (Serializable is logical) or a behavior

[sstok]

I vote for 0 or 4. I have no problem with the Interface suffix, but SourcesFromHistoryOfEvents just feels very strange to me.

[wjzijderveld]

I would go with 4 or current

But 👍 on documenting it, there probably will be discussions about this in the future... But nobody said naming things is simple :)

[asm89]

+1 on 4, though I'm not sure about the name ConventionBasedEventSourcedEntity. For the transition I think it would be interesting to do option 3 first and then one by one renaming things until we have option 4.

What do you guys thing? (cc @qandidate-labs/broadway)

[remi-san]

Hey everyone,

Has this been burried? Can we brint it back to life? as there seemed to be a consensus on approch number 4...

If so, tell me, I'd be happy to help and submit a PR on that matter.

[wjzijderveld]

Not buried, but a bit low on prio.

To be honest, I'm not sure what the best approach is here.
I'm afraid that if change all at the same time, the discussions are going to take forever 😄

[unkind]

It would be great to drop the suffix.

[unkind]

Not buried, but a bit low on prio.

Sorry for bumping it again, but wouldn't it be too late to rename interfaces later? This is a hard BC break.

[othillo]

we are about to tag 1.0, so I guess it's now or never time for this change.

Does anyone want to create a PR? It would be highly appreciated.

If not we should bury this as a known issue. 😑

[unkind]

@othillo I can try it. However, it makes sense to make decision regarding the following classes as they conflict with corresponding interfaces:

• class CommandSerializer -> NaiveCommandSerializer
• class EventDispatcher -> InMemoryEventDispatcher
• class DomainEventStream -> ArrayDomainEventStream
• class CommandHandler -> ?
• class Projector -> ?
• class EventSourcedEntity -> ?
• class CommandHandler -> ?

Do you really need CommandHandlerInterface, ProjectorInterface, EventSourcedEntityInterface, CommandHandlerInterface? The simplest solution would be just to remove them in favor of existing concrete classes. It is not that terrible as this convention probably reveals useless contracts.

[othillo]

after discussion with @wjzijderveld we decided on new interface names, see #308