Skip to content

Instantly share code, notes, and snippets.

@kcassam
Last active October 4, 2019 15:14
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save kcassam/8eb34332aecf0320a5a63cfc904a4a0b to your computer and use it in GitHub Desktop.
Save kcassam/8eb34332aecf0320a5a63cfc904a4a0b to your computer and use it in GitHub Desktop.
Weedo Best practices

General

You should know, have read and understood and those resources :

Please apply those best practices in your code when possible

PHP Quality library worth checking

Performance

How to tackle performance ? https://blackfire.io/ https://symfony.com/doc/current/performance.html

Weedo

Short best practice rules at Weedo

Rule #1 Enclose flush into try catch block

** UPDATE 2 September 2019 : Do not use flush (flush means transaction, either implicit or explicit, which are a hell to manage). See Rule #16. Rule #1 is left for legacy reason. If you use flush (which you should not), you still must follow rule #1.

You should always enclose a doctrine flush into a try catch block and at least log the exception message and trace

Don't forget that all subsequent flush will fail with a Doctrine\ORM\Exception "The EntityManager is closed" so take you responsabilities and act wisely

Here is a minimalist template to use when flushing

try {
    $em->flush();
} catch (\Exception $e) {
    $msg = '### Message ### \n'.$e->getMessage().'\n### Trace ### \n'.$e->getTraceAsString();
    $this->container->get('logger')->critical($msg);
    // Here put you logic now you now that the flush has failed and all subsequent flush will fail as well
}

Rule #2 Routes with trailing slash

Examples :

Yes :

frontend_page_digitalInStore:
    path: /{_locale}/digital-in-store/

No :

frontend_page_digitalInStore:
    path: /{_locale}/digital-in-store

Yes :

 * @Route("/photo/{id}/", requirements={"id" = "\d+"}, name="frontend_photo_show_canonical")
 * @Route("/{_locale}/photo/{id}/", requirements={"id" = "\d+", "_locale" = "en|fr|de|nl|ja|ko|zh_Hans|zh_Hant"}, name="frontend_photo_show")

No :

 * @Route("/photo/{id}", requirements={"id" = "\d+"}, name="frontend_photo_show_canonical")
 * @Route("/{_locale}/photo/{id}", requirements={"id" = "\d+", "_locale" = "en|fr|de|nl|ja|ko|zh_Hans|zh_Hant"}, name="frontend_photo_show")

Rule #3 Icons (frontend)

Do not use twitter bootstrap glyphicons. Use font awesome instead

Rule #4 Read the code

As a minimum, before coding in a Controller, read the corresponding Manager and Repository to read existing functions and not recode them. For example, before modifying PhotoController, you should at least read PhotoManager and PhotoRepository

Rule #5 No doctrine magic methods

You should not use doctrine magic methods for multiple reasons => use findOne instead of findOneByEmailCanonical and find instead findByEmailCanonical http://stackoverflow.com/questions/6184337/best-practice-php-magic-methods-set-and-get

Rule #6 Use 4 spaces for indentation

No tabs, no 2 spaces 4 spaces. You can set your code editor to type 4 spaces instead of tab when you type tab

Rule #7 : type hint as much as you can

  • tout est dans le "as much as you can" : parfois, ce n'est pas possible et parfois ce n'est pas souhaitable (dans ce cas, expliquer pourquoi dans un commentaire)

Rule #8 : no business logic in "Controllers"

No business logic in Controllers, Listeners, Commands, all those "interface" classes. Business logic should only be in services

read : http://mmoreram.com/blog/2015/08/20/re-thinking-event-listeners/

Rule #9 : use factory to create object

To create a business object, use a factory. Do not do the new Object yourself. If the factory does not exist, make it. See https://github.com/kamranahmedse/design-patterns-for-humans#-simple-factory

Rule #10 : When calling external API, you MUST do it via a SQS mechanism (hence asynchronous)

API example : Google vision, google guess language, google geolocalisation, update data on instagram, etc. Why : more easy to manage, no need to be in sync, with a fifo queue, you can deduplicate, better error management etc.

Rule #11 : Intl

Our Intl reference datas are here : https://github.com/symfony/symfony/tree/master/src/Symfony/Component/Intl/Resources/data

Use those data for :

  • currencies
  • languages
  • regions
  • locales (langues x regions)
  • scripts (alphabets)

For all you code, not only Php or Symfony code

Rule #11.a : corollary for locale field

7 characters is necessary and sufficient to store a locale value. See https://stackoverflow.com/a/38959322

Rule #12 : [PHP] when creating DateTime from string, use DateTime::createFromFormat instead of (new DateTime)

If you know the datetime format, use DateTime::createFromFormat instead of new DateTime If you do not know the datetime format of a date in a string, you really should ask yourself why.

En effet, (new DateTime) va essayer de transformer le string en date, quitte à faire n'importe quoi. DateTime::createFromFormat sera strict, et prendra le format fourni.

<?php
$date = "2018-02-07T00:00:00 0000"; // (Aïe : le '+' a été remplacé par un espace suite à un url encode !)
var_dump(new \DateTime($date)); 
// retourne object datetime avec date "0000-02-07 00:00:00.000000" !
var_dump(\DateTime::createFromFormat(\DateTime::ATOM, $date));
// retourne boolean false, c'est mieux

Rule #13 : Do not put nullable field in unique composite key

if one field is nullable, it means it can be null and the uniqueness is not garanteed in that case => apart from very marginal case, this will ruin uniqueness (for ex deletedAt in a key ruins it all)

Rule #14 : Use "calls" parameters over "arguments" for service config

example :

weedo.instagram.manager:
      class: Weedo\MediaSocialApiBundle\Entity\Manager\InstagramManager
      lazy: true;
      arguments:  #ok but not good
        - '@weedo.instagram.factory' 
[...]
      calls:  #good
        - [ setQueueWorker, ['@uecode_qpush.registry'] ] 
        - [ setApiManager, ['@weedo.instagram_api.manager'] ]

The reason why is that it is more readable and the class whitout the container is easier to use

Rule #15 : Filesystems I/O must use a Mediator Class.

Which implies for example, do not use directly Liip\ImagineBundle\Imagine\Cache\CacheManager::getBrowserPath, use FilesystemMediator::getUrl or FilesystemMediator::getCDNUrl

see : https://github.com/kamranahmedse/design-patterns-for-humans#-mediator

Rule #16 : Do not use flush. Use execute instead

Example (see ::safeMake)

<?php

class CaptionManager
{

    private $query;

    public function setEm(EntityManager $em) {
        $this->em = $em;
        $conn = $this->em->getConnection();
        $sql = "INSERT IGNORE INTO `caption` (`caption_id`, `text`, `user_id`, `created_at`) VALUES (:caption, :text, :user, :now)";
        $this->query = $conn->prepare($sql);

    }

    public function make(string $captionId, string $text, InstagramUser $user): InstagramCaption
    {
        return (new InstagramCaption())
            ->setCaptionId($captionId)
            ->setText($text)
            ->setUser($user)
            ->setCreatedAt();
    }

    public function safeMake(string $captionId, string $text, User $user): InstagramCaption
    {
        $userId = $user->getId();
        if (!$userId) {
            throw new Exception("CaptionManager::safeMake must be called with a persisted user");
        }
        
        $now = (new DateTime())->format("Y-m-d H:i:s");
        $this->query->bindParam(':caption', $captionId);
        $this->query->bindParam(':text', $text);
        $this->query->bindParam(':user', $userId);
        $this->query->bindParam(':now', $now);
        $this->query->execute();
        return $this->em->getRepository(InstagramCaption::class)->findOneBy(["captionId" =>$captionId]);
    }
}


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment