[3.0] Final classes by default

[unkind]

There is a rule in Symfony to make all non-public class members private by default, not protected:

We define all members as private until a valid use case is found that requires higher visibility.

by @jakzal (#14923).

I suggest to make BC policy more strict for Symfony 3: require final on every new class by default. Reasoning is simple: if you don't think that class should be inherited, then don't leave it opened for inheritance, it would allow to keep BC better. Sometimes I see PRs which technically break BC, but it is allowed because "it's too rare and weird case to inherit that class". To be consistent, you shouldn't allow to inherit that class then. Later you can remove final keyword if there is valid use case for inheritance.

I can refer to some similar opinions:

• http://verraes.net/2014/05/final-classes-in-php/
• http://ocramius.github.io/blog/when-to-declare-classes-final/

[fabpot]

👎

[Ocramius]

@unkind only for classes implementing interfaces and ONLY the interface methods as public API.

[xabbuh]

In my opinion we don't need to introduce this as general rule, but we could indeed spent some time about thinking whether or not a new class really deserves to be extendable (removing final is backwards compatibe).

[unkind]

@Ocramius yes, it can sound like that, less strict, but still reasonable. However, I usually mark almost every class in my projects. Why do you need to inherit value objects like RgbColor, for example? It has no separate interface, but leaving it open for inheritance is weird for me.

@xabbuh

In my opinion we don't need to introduce this as general rule

Yes, but at least marking as final shouldn't be discouraged:

we don't make classes final in Symfony generally

by @stof (#12115).

[Ocramius]

Why do you need to inherit value objects like RgbColor, for example?

In a framework, it is very hard to foresee what users are going to use your classes for.
If it's private and internal API (php lacks protected class scope, sadly), then yes, feel free to use final.
If the class doesn't match an interface (completely), and it is going to be used in userland, then don't mark it as final, as that's just asking for trouble.

[unkind]

If it's private and internal API

Symfony has @api tag for public classes/interfaces: http://symfony.com/doc/current/contributing/code/bc.html

IMO, it makes sense to allow final for:

• classes without @api tag;
• classes "implementing interfaces and ONLY the interface methods as public API".

[Ocramius]

@unkind that's good enough for me :-)

[fabpot]

Closing this generic issue. Let's add final where it makes sense.

[unkind]

Just to make sure our conversation from #19891 won't be lost:

@fabpot

@unkind I think that this is indeed a good conversation to have, but we should have it globally, not just for this PR (or any other ones). I'm not sold yet on adding final as it's a hard stop on inheritance and extensibility. I like the current way as we have a very clear BC promise (and we can probably add some specific notes about build-in commands, controllers, ...) but if people want to go wild, they can (knowing they are on their own).

@unkind

I like the current way as we have a very clear BC promise (and we can probably add some specific notes about build-in commands, controllers, ...)

This is not clear until you specify the list of classes which are risky to inherit. What does "want to go wild" mean? People should know what "wild" is. For some inheritance-oriented people extending listener/command is OK, they think it is very OOPish and, therefore, valid.

I think final is easier way to make BC promise truly clear.

I'm not sold yet on adding final as it's a hard stop on inheritance and extensibility

Yes, because you want to stop it on purpose, don't you? On other hand, you want to see private members by default, it leads to classes with no protected members at all. This policy is already "hard stop" on inheritance and extensibility: it makes those classes pointless to inherit.

Inheritance is not the only way to extend behaviour of the class. In most cases, you can fix your problem with composition.

Also, keeping classes open to inheritance blocks PRs like this one: #11708.

[naspy971]

Quite some years later, I find this post and I'm wondering why @fabpot said "Let's add final where it makes sense." and Symfony still does not implement this convention by default or gives an option to create classes with this keyword because I see many Symfony developers adding "final" themselves on controllers, services etc....

I might be missing something here.. ??

[fabpot]

Quite some years later, I find this post and I'm wondering why @fabpot said "Let's add final where it makes sense." and Symfony still does not implement this convention by default or gives an option to create classes with this keyword because I see many Symfony developers adding "final" themselves on controllers, services etc....

I might be missing something here.. ??

Two quick comments: the important part of the sentence is "where it makes sense", which means that the context is important and this should always be a conscious decision. Also, the rules that make sense for a framework like Symfony rarely make sense for applications.