Puissance 4 multi-joueur : retour d'expérience

Première semaine parmi les équipes marmelab, premier challenge de la période d'intégration, 4 jours pour mettre au point un serveur de jeu puissance 4, c'est parti !

Le sujet : 2 joueurs se connectent sur un site web et se rejoignent pour une partie de puissance 4. A tour de rôle, ils déposent un pion jusqu'à ce que l'un des deux gagne ou que le plateau soit rempli. Les technos : PHP7, Symfony 3.

Les objectifs ? Se familiariser avec les méthodes et les technologies marmelab en soumettant son travail à l'avis de ses pairs.

Qu'en ai-je retenu...

L'art de la sémantique

Premier choc avec l'exigence marmelab, la sémantique ! La plupart des remarques dans mes revues de code portaient sur le nommage des différents concepts du jeu. J'ai toujours été sensible au nommage dans les développements en essayant d'être au plus proche du langage réel, mais dans ce cas, c'est d'un autre niveau ! Chaque classe, variable, méthode, argument doit être nommé de façon à ne pas avoir besoin de commentaires, la règle d'or chez marmelab : s'il y a besoin de commentaires, c'est que le code n'est soit pas assez clair, pas suffisamment bien nommé ou pas assez éclaté.

Une fois un peu familiarisé avec les méthodes de nommage, le nombre de revues a diminué, mais on peut aussi se faire avoir lors d'un refactoring !

class Board
{
    const COLUMNS = 7;
    const ROWS = 6;

+   private $cells;
-   private $discs;

    public function __construct()
    {
        $this->initializeCells();
    }

Dans le modèle initial, le plateau de jeu contenait un tableau de pions (discs) puis, suite à un changement de logique, une matrice de cellule; chaque cellule pouvant contenir un pion ou être vide.

Lorsqu'on remet en cause la signification d'un modèle de données, il ne faut pas oublier de se poser à nouveau la question de son nommage. Le nom discs n'est plus du tout adapté à ce que le concept représente, il pourrait donc être mal interprété par un futur développeur et amener à compromettre la maintenabilité générale du projet.

Après concertation avec les reviewers et proposition de plusieurs noms, celui de cells a été retenu. Le nommage doit être compris par tout le monde et je pense que le fait de s'appuyer sur le maximum de personnes aide vraiment.

Best practices

Conventions & cohérence

Le style de code, l'indentation, le nommage... on a tous des habitudes différentes et ça ne facilite pas le travail en équipe. On a aussi tous un jour rêvé d'avoir des développements complètement uniformes quel que soit le développeur.

Chez Marmelab, on cherche à accorder nos styles au maximum ! Et afin d'être le plus standard possible (open source oblige), on s'appuie sur les conventions en place dans les technologies utilisées, par exemple avec PSR-2.

Des outils comme php-cs-fixer peuvent aider à appliquer ces conventions.

Simplification

En plus d'avoir un code uniforme, on cherche aussi à avoir un code simple, et c'est pas si évident ! Marmelab essaye d'éviter les machines de rube goldberg, sauf lorsqu'il s'agit de partager sa connexion en fabriquant une antenne wifi avec des boîtes de conserves...

Pour que n'importe qui puisse maintenir le code facilement, même règles pour tout le monde, on se débarasse de tout code superflu.

Quelques exemples :

• Le else de cette fonction n'a aucune valeur ajoutée :

    public function getStatus() : string
    {
        if ($this->isFinished()) {
            return self::FINISHED;
        } elseif ((bool) $this->yellowPlayer && (bool) $this->redPlayer) {
            return self::PLAYING;
-       } else {
-           return self::WAITING;
        }
+       return self::WAITING;
    }

• Le bundle SwiftmailerBundle n'est pas utilisé est peut donc être enlevé:

    $bundles = [
        new Symfony\Bundle\FrameworkBundle\FrameworkBundle(),
        new Symfony\Bundle\SecurityBundle\SecurityBundle(),
        new Symfony\Bundle\TwigBundle\TwigBundle(),
        new Symfony\Bundle\MonologBundle\MonologBundle(),
-       new Symfony\Bundle\SwiftmailerBundle\SwiftmailerBundle(),
        new Doctrine\Bundle\DoctrineBundle\DoctrineBundle(),
        new Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle(),
        new AppBundle\AppBundle(),
    ];

• Le commentaire phpdoc ne sert pas puisque notre code est auto-explicatif :

-   /**
-    * Get id.
-    *
-    * @return int
-    */
    public function getId()
    {
        return $this->id;
    }

Des tests, des tests, des tests !

Chez marmelab, tout doit être testé ! Dès qu'une nouvelle classe ou une nouvelle page est ajoutée, on ajoute les tests qui vont avec en même temps, et ils intègrent le même commit ou la même PR.

Le prochain développeur pourra s'assurer qu'il n'a pas cassé notre code sans avoir à tester à la main et sans avoir à comprendre le fond de tous les développements effectués.

Les tests, ça sert aussi de documentation, le fait d'utiliser les méthodes ajoutées dans les tests fournit de bons exemples d'utilisation aux prochains développeurs souhaitant l'utiliser.

Comme l'execution de ces tests automatisés est une phase critique du développement, on configure même un serveur d'intégration continue qui les exécute sur chaque push, chaque PR. On est sûr que les tests passent avant de merge, et si on a une bonne couverture de code, on est beaucoup plus serein sur les non-régressions.

Partage

Dans le développement classique, on est habitué à respecter les spécifications et à ne montrer son code que lorsqu'il est vraiment prêt à être montré.

Marmelab encourage à montrer du code fonctionnel rapidement afin d'être sur une base de cycles très courts et de détecter et corriger les imperfections au plus vite. Ce principe s'applique via des revues de code systématiques (à l'aide de pull-requests) sans spécifiquement attendre que tout soit terminé : "à la MVP".

La granularité de ces pull-requests devient alors assez critique puisqu'il faut trouver le juste équilibre entre une fonctionnalité montrable et une quantité de code à revoir restant mesurée.

J'ai pu voir qu'il fallait bien séparer les PR par fonctionnalité, qu'il ne fallait pas hésiter à prendre des raccourcis et à envoyer du code, même partiel, pour qu'il soit validé. C'est aussi important de ne pas tout mélanger, les reviewers doivent savoir ce qu'ils sont en train de valider. Par exemple, un commit d'adaptation de l'architecture au milieu d'une PR concernant une user story ne fera qu'apporter de la confusion.

Du temps presque réel

Aspect non négociable pour ce projet: réussir à faire du temps réel avec une expérience acceptable à l'aide des technologies qu'offre Symfony.

Première étape : Le temps réel du pauvre

Dans le cadre d'un développement rapide, un simple rechargement de la page est un raccourci assez efficace puisqu'il permet en quelques lignes d'implémenter un rafraichissement dégradé qui fonctionne.

Le reload permet de recharger l'état du plateau toutes les 2 secondes sans faire de différence avec le chargement initial de la page.

<script>
  setTimeout(function() {
    window.location.reload();
  }, 2000);
</script>

Ca provoque du flicker toutes les 2 secondes et l'expérience utilisateur est vraiment pas top, mais ça fonctionne !

Deuxième étape : Le temps réel avec rafraîchissement partiel

Tout en vanillaJS, à l'aide de l'API fetch d'HTML 5 et d'un setTimeout récursif :

function refreshBoardContent(boardElement) {
  // [...] some parts omitted for brevity
  return fetch(
    Routing.generate("viewBoard", {
      id: gameId,
    }),
    {
      method: "get",
      credentials: "include",
    }
  )
    .then(function(response) {
      if (response.status == 200) {
        return response.text();
      }
    })
    .then(function(data) {
      if (data) {
        boardElement.innerHTML = data;
      }
      setTimeout(function() {
        refreshBoardContent(boardElement, recursive);
      }, 2000);
    });
}

Note: L'ajout d'un polyfill de fetch est malheureusement nécessaire pour que celui-ci fonctionne sur Safari.

Il faut aussi gérer le cache et son expiration côté serveur afin d'éviter que la même vue ne soit envoyée à l'identique toutes les 2 secondes.

<?php
public function viewBoardAction(Game $game, Request $request)
{
    $game->replayMoves();

    $response = new Response();
    $response->setETag(md5(serialize($game)));

    $response->setPublic();

    if ($response->isNotModified($request)) {
        return $response;
    }

    // [...]

    return $this->render("game/$view.html.twig", array(
        'game' => $game
    ), $response);
}

Un Etag de l'objet est donc calculé avec md5 et permet de savoir si celui-ci a été modifié depuis le dernier chargement. Si ce n'est pas le cas, Symfony renvoi une réponse 304 NOT MODIFIED vide.

Conclusion

Première semaine en immersion, ça change des méthodes "traditionnelles" ! Marmelab est une société, une équipe, où le bon sens prime. On s'impose des règles en gardant à l'esprit leurs raisons, on s'accorde des raccourcis sans transiger sur la qualité et surtout on profite au maximum de l'intelligence collective.

Hâte de passer au prochain challenge !

Note: Le code du serveur de puissance 4 développé en 5 jours par Florian est disponible sur GitHub: marmelab/connect-four.