Search code examples
phpsymfonysonata-admincode-standards

Is it good practise to extract strings from framework standard syntax?


For example: In sonata admin, a single admin class has always recurring syntax like

$formMapper->add('test', null, ['label' => 'testlabel']);

$formMapper->add('test1', null, ['label' => 'testlabel1']);

$formMapper->add('test2', null, ['label' => 'testlabel2']);

In this case if i am adding several fields with the add method, our quality gate is telling me, that it is a code smell using the 'label' string more than 2 times. I should add a constant for that string ...

Is it now good or bad practise to have many many classes full of constants like

const KEY_LABEL = 'label'

$formMapper->add('test2', null, [self::KEY_LABEL => 'testlabel2']);

???

I can't figure out, whats the great benefit .. if someday the label key will change, which is given by the framework, i have to make changes to this stuff either way ...


Solution

  • Short answer: No. Symfony doesn't rename keys.

    Long answer: Symfony has a pretty strict Backwards Compatibility Promise. It is not very likely the label key will be renamed. But that doesn't mean that it won't change.

    If you have to 'rename' a key, it is very likely you have to change the value too. That means you're not renaming, but replacing it. Using a constant will help you rename the keys, but it won't help you refactoring the value.

    I started working with Symfony many years ago (started with 2.1 and testing 3.4-beta right now) and I've had to perform some 'renaming' in the past. In almost all cases the key wasn't renamed but it was deprecated and later replaced by another key. One of the examples I can remember is the Choice form type refactorization, but I'm sure there are more and even better examples.