Analysis based on Symfony Best Practices 2.7 during our InnoTalks #7

Purpose 

This article was written based on the internal debate we, at Innobyte, had on some of the recommendations presented in the Symfony Best Practices guide (referred to as SfBP from now on), some of which are different than what most expected, and some of which bring confirmation to our beliefs, but are subject to debate amongst developers. So, I skipped the topics that seemed trivial to the common sense.

To join future debates, follow our social media channels and check for messages that announce InnoTalks, our periodic meetings/presentations held by Innobyte’s team.

Now, back to our Symfony Best Practices debate from our last InnoTalks session.

What is a “best practice”?

To sum up, a best practice is an optional recommendation, which consists of a well defined procedure that is known to produce near optimum results. Why near optimum? Because, in the real world, pursuing absolute perfection can prove to be, sometimes, very inefficient.

Who is this article for?

This article is intended for Symfony developers, with (at least) a basic knowledge of the framework and concepts behind it.

Pre-requisites

This article is an analysis of the Symfony Best Practices guide. So, I assume you have already read http://symfony.com/doc/2.7/best_practices/index.html. If not, please read it before continue reading this, as this article does not re-explain the reasons behind some of the recommendations, it just comments on them.

Organization

What is a Bundle?

A bundle is something (a collection of code) that is meant to be used as a standalone component, which may be used in a number of different projects, with only modifying some configuration options.

1. In relation to code organization, SfBP recommends having only one bundle inside the source directory of the project (src ).

This sounds like a good idea for small and medium projects, but from my experience, in large projects, having multiple bundles can bring a well needed sense of organization and could also help in the separation of concerns.

In my opinion, bundles inside src  are by nature connected, because they compose the source of the project.

Really decoupled bundles should stay outside of src , extracted into separate packages and installed through composer. However, this comes with a disadvantage: making modifications inside an external package takes more time, since you have to separately commit and tag releases, then update the dependency inside the project.

The recommendation of having just one bundle is also acceptable as a compromise for large projects, since things can be organized using namespaces (and folders) inside the AppBundle .

Eg: @AppBundle\Entity\Checkout\Order instead of @CheckoutBundle\Entity\Order

So, is this an architecture change? Why was the src  folder meant to have multiple bundles?

Why doesn’t the architecture force to have only one top-level inside src  (and eliminate the concept of bundles from src , and keep it only in vendor )?

A possible answer would be that you could really have totally decoupled bundles inside src , and choose not to version them because of the involved overhead brought by external packages. But this isn’t a really good reason to do so.

Another possible reason is that bundles from vendor  folder may be inherited (and overridden) inside src .

Ultimately, this is a matter of taste. This topic was the most intense during the Best Practices debate; some of us prefer one method, some of us, the other. As long as the organization is not lost, by using sub-folders (and namespaces), all is fine.

2. Prefixing the bundle name with the vendor isn’t necessary inside src .

This recommendation is really a welcomed one, because I often saw this unnecessary step being taken with no exception.

Configuration

3. Class constants can be used to keep not-so-configurable values, meaning values that may change several times during the lifetime of a project.

This could save some work when it comes to injecting some of the configuration parameters into various components, such as Forms or Repositories (without defining these as services).

4. Also, it is advised not to use semantic configuration for own bundles (source bundles).

Agreed, documenting the parameter values constraints could be enough for own services, and saving some time is always welcomed, since this time could be invested in something else, such as unit or functional testing.

However, depending on how long it would take to build it, it may be worth having it, since it is a pretty nice feature – this depends on the number of parameters and the conditions, and, of course, on the experience of the developer with semantic configuration. Also, the number of developers involved is a factor, and also is the mechanism of knowledge sharing inside the project: better communication, less validation mechanisms.

5. Not a Symfony-related recommendation, but a security best practice, DO NOT VERSION CREDENTIALS.

A (sadly) very common mistake is to version Database or API credentials for third party system access. This can compromise your project in a number of ways.

One good alternative would be to use environment variables, which are external to your project. Also, most cloud systems play well with this approach. Other approaches may include using Jenkins (or Capistrano) to replace some tokens inside versioned files, by a post-deploy hook.

Business logic

What is the business logic?

The business logic is the code that is written for the application and is not framework-specific (some framework-specific examples are routing and controllers).

6. SfBP confirms that files may exist outside of bundles.

Until now, many developers thought that anything inside src  folder should live in a bundle.

So, if you have code that could be organized in a library, go for it. You can make the library lay in its own folder, then build a bridge bundle to integrate it into Symfony. The advantage of doing so is that the component you build is not coupled to Symfony framework and could be reused elsewhere.

It also provides an extra abstraction layer.

7. SfBP recommends that the names of the services from within src  should be as short as possible, with no prefix.

While this sounds logical, this is different that what we’re used to up until now. Also, it’s different than what examples showed us, and also different than the recommendation found in http://symfony.com/doc/current/cookbook/bundles/best_practices.html:  “If the bundle defines services, they must be prefixed with the bundle alias.”

But besides this conflict in recommendations, from experience, not all third-party code follows best practices when it comes to naming conventions, so it might be a good idea to prefix your services with a very short name, built at least from 2 or 3 characters, to avoid naming conflicts and also to help you identify own services real quick (this can become a time-saver in very large projects).

8. The YAML format is recommended for defining services, because of its simplicity.

There is no overhead in using one format or the other, all have the same performance, because they are compiled when the container is built.

I also prefer the YAML format, because it is easily readable, but this is a matter of each individual’s preference.

However, XML provides an advantage: PHP class constants can be used inside them.

9. SfBP recommends that service classes should not be defined in container parameters, but instead written directly inside the service definition for src  located services.

However, such an approach can make life easier, by overwriting service classes depending on the environment. It may sound like a wrong approach, but it can be necessary; example: Traceable things for dev environment.

10. Annotations are the recommended choice for defining Doctrine entities, for both flexibility and being close to the things they define. Again, no performance impact is present depending on the chosen format.

This sounds like a great idea, with the condition of having relatively short entities. From experience, when entities become large (more than 20 properties), annotations can become harder to read than an YAML.

But wait, entities shouldn’t grow that big! They should be split after a certain point into more related entities. Ideally, yes. But, unfortunately, this is not always easily possible, since many blocks of code could need to be updated, which could cause regressions, and also, there is the issue of having downtime while executing some SQLs to migrate the data and alter tables (which cause locks).

11. SfBP recommends DoctrineFixturesBundle  to insert fixtures, but also recommends having just one (yes, one!) class for these.

Personally, I prefer to have things very well organized, and disapprove this one. Every type of fixture should have its own class. So, one class of fixtures for each entity is what I prefer.

12. Coding standards PSR-1  and PSR-2  are recommended.

Everyone should adhere to and respect coding standards, and nowadays this is a very easy thing to do, since IDEs have auto-formatting features.

Controllers

13. The philosophy “thin controller, fat model” is embraced by SfBP, which is something we’d expect. The 5-10-20 rule is recommended (5 variables per method, 10 methods per class and 20 lines per method).

While 5 variables is a low count for a method, and large projects could exceed the 10 methods per class rule in order not to over-fragment things, the point is clear: separation of concerns needs to be followed, because building concise methods greatly improves readability.

So, while the target is not quite feasible in real life, we need to aim for it in order to obtain quality code.

14. Annotations are the choice of SfBP for defining routes, in order not to spread the configuration options amongst multiple files.

These are great for most of the cases, but when configuration options pile up, like when using FOSRestBundle  for example, it may be a good idea to use YAML instead.

Also, if routes change and the order needs to be changed between routes, this is way more easy to do with YAML.

15. Regarding the separation of concerns and building decoupled software, SfBP recommendation is to keep the business logic decoupled from the framework, while the coupling between controllers/routing and framework may be maintained.

This sound perfectly logical, since the routing system is deeply connected with the architecture of the framework.

16. Any kind of logic should be extracted from controllers, and delegated to services, including common CRUD operations.

This brings benefits in terms of separation of concerns and makes the code easier to tes, but for trivial operations, this can become an unnecessary overhead.

17. A recommendation not to use the @Template  annotation is issued, for performance reasons.

While it may seem elegant in approach, the mechanism that is triggered when the controller does not return a Response (exception thrown and caught by a listener, plus determining the template file to render) takes time; approximately 20ms, according to SfBP.

So, it is a perfectly objective reason not to use it.

However, when building an application that involves an API and also a website that use the same controllers, and wish to totally decouple the presentation layer (like when using FOSRestBundle ), this is unavoidable.

18. An encouragement is issued to use the @ParamConverter  annotation (implicit or explicit).

This is a great idea, and we’re also using it whenever possible. Please take a look into it if this is something new for you:
http://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/converters.html

Templates

19. The recommendation made by SfBP is to store templates under app/Resources/views/ .

The reason behind this is that the names are shorter, so, there is less to be written. Also, the justification is that designers don’t have to look under multiple bundles for templates.

I personally don’t like this approach. The reason that the templates names are shorter is not nearly enough reason to move the templates outside the bundles. It seems to me that the templates should be located on the same level as those that render them (Controllers), so they should lay inside bundles. Doing otherwise breaks the beautiful way of organizing things that Symfony offers.

Regarding the second reason, since SfBP recommends a single bundle, this shouldn’t be an issue.

Also, the naming convention in Symfony is crystal-clear, so there can’t be any confusion regarding where to look. Furthermore, by using a powerful IDE, with a plugin for Symfony, things get even easier.

One advantage could be that a file seek is saved when building the cache, but this is totally negligible.

The only templates I prefer to store under app/Resources  are the base templates, because they are used (extended) by more than one bundle, but if following the single AppBundle  recommendation, the base templates can also be moved inside this one.

20. The snake_case  must be used for template names and folders.

A convention is a convention. 🙂

21. Twig extensions must be declared inside AppBundle/Twig/, which sound very good, but their definitions must be located under app/config/services.yml. At least, this is the recommendation made by SfBP.

I prefer to define these services (tagged as Twig extensions) in each bundle, since they are ultimately services; to me, I it seems like a better approach to keep similar things on the same level.

Forms

22. The use of separate classes for forms (subclasses of AbstractType) is encouraged, but single-use forms may be build on-the-spot in controllers.

I totally agree with this approach. Simple forms, like a delete form (containing just one auto-generated CSRF token filed), or perhaps an additional “id” field, does not need to have a class of its own. It could stand in a method inside a Controller class.

23. SfBP recommends adding buttons inside templates (as HTML code) instead of adding them in the form class, in order not to couple the form with the place in which it will be used.

I prefer to add buttons in the form, with a generic name, such as “submit” and “cancel”, or even specific in some cases (“next” or “previous”), and then set the needed labels in the template.

In fact, I prefer to set ALL labels in templates (and other presentation-related attributes, such as “id” and “class”), for all elements, since I consider this to be a presentation task.

This also is an advantage when having to check if a certain button was clicked.

24. A single controller (“action” method) is recommended to be used for both rendering the form (on GET) and processing its submission (on POST).

Ex: new /create . This is a really great approach, which I adopted from the beginning.

But this is in contrast with the way the code generator worked until now – it generated separate controllers (actions).

25. The explicit check of $form->isSubmitted()  is recommended for clarity, although $form->isValid()  also makes the submitted check.

I totally agree this kind of suggestions, as I am an advocate of easy-to-read and verbose code. And clarity of code is, unfortunately, well underestimated by many.

Internationalization

26. The XLIFF format is recommended for storing translations, due to the great variety of professional software that is available for editing these files.

However, big projects will probably need a custom solution for loading translations, like from the Database.

27. The recommendation is to store the translations in the app/Resources/translations  folder, because they need to be centralized.

This is a recommendation I do not plan on adopting, along with the other recommendations for storing bundle-specific resources under app/Resources  folder. The reasons are already stated above.

28. Translation keys are preferred over sentences/phrases.

The benefit is clear for any experienced developer: when changing a phrase, you do not need to keep the key, so the danger with ending up with translations like “Thank you for your shopping session” having the key “You have been logged out” is mitigated. Only choosing appropriate key names eliminates this completely (see below point).

Also, they are easier to read, since a key may have up to 20 character, while a sentence may have hundreds.

29. Completing the above-mentioned recommendation, the keys should describe the purpose of the translation instead of the place in which it is used.

So, a key named edit_form.username  is not a good choice, since it may be used also on the create form.

Security

30. A single firewall with anonymous: true  is enough for most applications. To be more precise, a single firewall per real user provider is recommended.

From experience, I can say that this is a great recommendation. I have seen applications with web interfaces and APIs (both mobile and used in communicating with other internal apps) having different firewalls and this setup can rise an unnecessary set of issues.

So, if you have a mobile API, on which same users as on the website authenticate, use a single firewall for both.

If you have an API on which third-parties authenticate (different user set than on website), go for separate firewalls.

31. bcrypt  encoder is recommended.

The hash includes the salt, the cost of the password encryption is adjustable (more computing power could mean more secure passwords – in the same amount of time), and it is resistant to rainbow-table based attacks.

32. The security must not be over-complicated:

• Use access_control  for securing broad URL patterns.
• Use Security Voters for general rules that can be enforced by programmatic algorithms.
• Use ACL (Access Control Lists) for enforcing rules defined in a totally custom manner, to grant access per object, per user.

33. Use @Security  annotation whenever possible (for simple rules). The only disadvantage is that the code must be duplicated wherever it is used.

The authenticated User  is available, along with any variables that are used with @ParamConverter . Also, expressions are supported.

Otherwise, use the security.context  / security.authorization_checker services  (depending on what version of Symfony you are using).

34. Good news for simple checks: entity-based methods that perform checks on passed objects are OK to use.

This is a controversial subject, which has been considered a bad practice by many (including me) up until now.

Assets

35. Assets are recommended to be stored directly in the web  folder.

One recommendation on which I also don’t adhere to. These are also resources, and should lie in the Resources  folder inside each bundle.

Again, the reason of reducing the spread between bundles is the presented reason, along with the names being shorter.

A major disadvantage is that this way the sources are publicly available, along with comments that could expose sensitive information. Avoiding this would require extra work, such as building a post-deploy hook to delete the source files.

36. Asset combining and minifying using Assetic  is encouraged.

This will result in fewer requests for serving wp-content and also saved bandwidth, which ultimately will translate into the website load time decrease, and a better experience for the users.

37. Assetic  can also compress images.

Since this is a one-time operation for most of the files, it seems unnecessary to process these images at each build.

Also, an optic check (made by a human) seems mandatory to me, since compression for an image depends strongly on its content.

Using a filter to produce sprites, on the other hand, is a very effective way of optimizing images used in the graphics of the website, resulting in a reduced number of requests, but can slightly increase bandwidth, since you may download more images than you need to.

Testing

38. Unit testing is used for testing the business logic (which should be decoupled from the framework), so the classes used to test this should be independent of Symfony. PHPUnit  is used for this.

39. Functional testing is a must-have for any project, however simple they may be.

I personally greatly appreciate the example that is presented, in which only the success status code of the response is tested; while admitting it is a very simple test, SfBP puts the emphasis on how easy it was to create it and highlights the benefit/effort ratio, which, under these circumstances, cannot be contested.

40. Hardcoding the URLs in functional testing is very useful.

While it might not seem obvious, it is a very valid point of view. Changing URLs may have a devastating impact on SEO (by breaking the URLs), and also may negatively impact the users (by braking the Bookmarks). By using the URL generator to obtain the links, changing these without intent may slip unnoticed.

41. Faker  and Alice  are recommended for generating fixtures.

42. Mink  is recommended for testing javascript through PHPUnit .

43. If the frontend application is javascript-intensive, such as an Angular  one, the recommendation is to use something else altogether, like GruntJs .

Conclusion

My personal conclusion is that, while I am slightly disappointed by the looser organization of the resource files (basically, the Resources  folder inside AppBundle  has become deprecated), this guide is a valuable asset in the Symfony community.

It has confirmed many of the things we do are good, but has also has made us question a couple of the approaches we’ve taken so far, like semantic configuration for own bundles or avoiding to use entity-based checks. I also appreciated the references to the Demo app, and there are quite a few.

Writing less code with loosing something (in this case, organization) will never be a good enough reason for me, as I believe the time saved is negligible, so I do not approve the reasons for deprecating the Resources  folder inside own bundles (located in src).

Code clarity and organization is very important to me and this is one of the things I’ve loved about Symfony right from the beginning. It would be a shame to diminish this now.