ConnectionManager::destroy & bulk

[armetiz]

Pensez-vous qu'un utilisateur puisse se retrouver avec une ensemble de Connection à supprimer ?
Si tel est le cas, il faut que l'on change le prototype de ConnectionManager::destroy.

Actuellement

public function destroy(ConnectionInterface $connection)

En (solution 1)

public function destroy(array $connections)

Ou (solution 2)

public function destroy($connection) //array ou ConnectionInterface

Personnellement, je ne suis pas fan des paramètres dont le type est mix et je vote pour la solution 1.

[choomz]

Je n'ai pas d'avis là dessus. Si tu regardes les méthodes update() et destroy() des repositories, elles utilisent la solution 2, avec une méthode intermédiaire persistConnection ou removeConnection qui type l'entrée. Impossible donc de persister/supprimer un mauvais type etc ..

C'est une pratique courante dans doctrine et ça facilite la vie, sans être à mon sens stupide.

[armetiz]

La si tu balance un $repository->update(new \stdClass()); tu aura une erreur PHP qui sera générée par une méthode protégé de Repository.
En interne, ok. Mais sur l'utilisation externe.. Je n'en suis pas fan.

$connectionManager->destroy(new \stdClass()); et l'utilisateur chopera une erreur qui viendra d'une méthode protégée d'un Repository..
En tant qu'utilisateur, quand je me plante sur le fonctionnement des librairies et que je me chope des erreurs provenant des bas fonds de la librarie.. Je trouve cela peu sexy de remonter le stack pour voir d'où provient mon erreur..

Après, à l'utilisation, c'est en effet plus pratique de pouvoir balancer une ConnectionInterface ou un array directement, je suis d'accord.

[choomz]

Oui c'est ce que je dis, persistConnection/removeConnection balancera une erreur sur le type.

[armetiz]

Je me suis mal exprimé.
Pour moi l'utilisateur doit récupérer des erreurs provenant du ConnectionManager, et non des RepositoryInterface.

Du coups,
Cela signifie faire des vérifications de type au sein du ConnectionManager et soulever des InvalidArgumentException au besoin.

[choomz]

Et bien dans ce cas, conservons le destroy(ConnectionInterface $connection) et rajoutons un destroyBulk(array $connections) dans le manager.

[benjamindulau]

Un petite interrogation.
Etant donné qu'une connection fantôme ne peut à priori plus exister, et la méthode destroy n'étant utilisée plus que dans le Manager lors des disconnect*, pourquoi ne pas la laisser inchangée et la passer en protected ?

[choomz]

Alors de mon coté, le destroyBulk ne me servira pas je pense, mais ça peut être utile de l'implémenter.

Par contre pour le destroy, je pense qu'il faut le laisser en public, car dans un traitement ou l'on doit récupérer des connections, boucler, etc ... il peut être intéressant d'appeler directement destroy() plutot que de passer les nodes correspondantes à la connection. Ca a un coté pratique je trouve.