Skip to content

[RFC] Rename Translatable to better follow conventions #38342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
dborsatto opened this issue Sep 29, 2020 · 8 comments · Fixed by #38424
Closed

[RFC] Rename Translatable to better follow conventions #38342

dborsatto opened this issue Sep 29, 2020 · 8 comments · Fixed by #38424
Labels
Translation

Comments

@dborsatto
Copy link
Contributor

Description

The recently-introduced Translation\Translatable class is a fine addition, but in my opinion it's quite oddly named. Specifically, the *able suffix is usually employed for interfaces (for instance Throwable and Stringable built-in in PHP, and HttpKernel\RebootableInterface in Symfony) and occasionally with traits (like with Command\LockableTrait). Especially lately with the push for removing suffixes and prefixes (like *Interface, *Exception and Abstract*) I believe the name Translatable could result in some confusion in whether we are dealing with a class or an interface. I am aware that TranslatableInterface has just been added, but I think that strengthens my point as that seems to be a much more natural way to use "translatable".

The conventional wisdom is to use actual nouns for naming classes (either describing what they are, as in Exception, or what they do, as in Form\FormBuilder), while adjectives are reserved to interfaces as their purpose is to describe a property or behavior that the concrete class is supposed to have. The name Translatable for a class breaks these general assumptions, and from what I can see this would be something that is not used elsewhere in Symfony, therefore making it an odd case that sticks out. I'm aware that there is an argument for nominal adjectives, but in English pretty much any adjective can be turned into a noun yet that's something we (as developers) don't generally do because it kind of goes against the norm.

My proposal would be to rename Symfony\Component\Translation\Translatable in order to follow the conventional wisdom and style used elsewhere in the project, and now would be the time for doing so as it has not yet shipped in a final release. The simplest way to change it would be something along the lines of TranslatableMessage, but of course there can be many alternatives.

Cheers
@ro0NL
Copy link
Contributor

ro0NL commented Sep 29, 2020

now there's an interface, i agree describing the implementation makes more sense.

But implying the default translatable is a translatable message string, still works for me as well :) grammar-wise it's fine. If it breaks "The conventional wisdom" i think we should rename it yes 👍

@natewiebe13
Copy link
Contributor

The original PR used TranslatableMessage. I think we should avoid just using Message to avoid conflicts with Symfony Messenger. Phrase could also work as well.

@dborsatto
Copy link
Contributor Author

Do you think TranslatableString suffers from the same issue, related to the string component?

@natewiebe13
Copy link
Contributor

@dborsatto I don't think so, I'm just thinking in regards to if the class name is the exact same, making sure to pick the right one (like Response from HttpFoundation) and having to potentially use aliases. Also just realized it's the Mime Component that has the Message class.

After my last comment, I also though about TranslatableString and TranslatableText, but I think I still prefer TranslatableMessage because it constitutes more than just a string.

@noahlvb
Copy link

noahlvb commented Oct 4, 2020

How about something different like Translation because a translation is Translatable hence the interface. This I think is in line with the naming conventions and does not clash with existing naming as far as I know

@natewiebe13
Copy link
Contributor

@noahlvb I did think about that one as well. The object isn't actually a translation as that is supplied by the translator. The object that needs to be named is the representation of a message and contextual data that will be used to get a translation at some point later on.

@natewiebe13 natewiebe13 mentioned this issue Oct 5, 2020
Merged
@natewiebe13
Copy link
Contributor

Opened #38424 in anticipation of changes.

@romaricdrigon
Copy link
Contributor

When using for the first time the feature, I also found Translatable to sound not quite right.

Translatable is definitely a great name for the interface, as we have other *able interfaces in PHP.

I have mixed feelings about TranslatableMessage. have the impression that for Symfony Translation component message are the values in translation files, for instance the default name ie called messages. So, to rephrase your last comment, it is a message + some context.

Finally, from a user of the framework point of view, I will use this class to get something translated.
What about ToTranslate, then?

@fabpot fabpot closed this as completed Oct 7, 2020
fabpot added a commit that referenced this issue Oct 7, 2020
@fabpot
feature #38424 [Translation] Rename Translatable class to Translatabl…
b0ae228 
…eMessage (natewiebe13)

This PR was submitted for the master branch but it was merged into the 5.x branch instead.

Discussion
----------

[Translation] Rename Translatable class to TranslatableMessage

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #38342
| License       | MIT
| Doc PR        | TBD

This PR is in anticipation of #38342 passing. We'll need to also update the docs and the blog post here: https://symfony.com/blog/new-in-symfony-5-2-translatable-objects

One extra note not discussed in the issue thread is the increase in the class name length shouldn't really be a problem as `t()` is likely to be used more often.

Commits
-------

0d4e25f Rename Translatable class to TranslatableMessage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Translation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Translation] Rename Translatable class to TranslatableMessage natewiebe13/symfony
7 participants
@fabpot @dborsatto @natewiebe13 @romaricdrigon @ro0NL @xabbuh @noahlvb