check for appropriate separation of constants about yoastcs HOT 3 CLOSED

@increddibelly Is this new ? I can't remember ever having come across the src/constants folder in the Yoast plugins before.

Other than that:

• What about the constants like PLUGIN_DIR and such which are normally created in the "main" file of a plugin ?

• if the constant is private (7.1) or in other words, if the constant in a class are only used in that class and its unit tests.

  This seems like a contradiction in terms.

  • If a constant is private it cannot be used in the unit tests for that class.
    While this wouldn't be enforced until the private modifier would be added, the principle of this is plain impossible.
  • Class constants ALWAYS have to be declared in the class.
    Introducing global constants in favour of class constants sounds like a really bad idea.

from yoastcs.

Just read the section in the docs, so we're talking class constants in a multitude of class in the /src/constants folder if I understand this correctly ?

I still wonder about why these constants would be disconnected from where they are used. Wouldn't it be more appropriate to add them to an applicable interface in that case ? (and yes, interfaces can have constants)

from yoastcs.

correct, the intent is to bundle constants per domain in a class in the src/constants folder. This is still a PR but I'd love to have a code sniff (warning) for this, as it helps people migratie toward the desired situation.

The main reasoning behind this is to reduce coupling between classes. e.g. we had an integration class that used a constant from another integration, which created a hard dependency between two logic classes that are in totally different domains. Through moving the constant into a separate class, the dependency becomes a much cleaner triangle like this: integration a -> depends on -> const class, and integration b -> depends on -> const class.

we discussed keeping the constant near the logic using it, but since the rest of the code is divided into folders based on their design (repository / presenter / etc) we decided to move the constants to a constants folder, to conform to the rest of the folders.

the exception:
if a constant is only used privately, we could mark it as private constant but that's not OK for PHP 5.6. So, they need to be public. We felt it's ok to keep the constant in the class using it. As soon as another class needs that const, the const needs to move to the src\constants\ folder.

TLDR it's too bad PHP doesn't support ENUMS because that's what we're trying to do here 🙃

from yoastcs.