Concevoir des méthodes liées à la base de données, qu'il vaut mieux renvoyer: vrai / faux ou ligne affectée?

10

J'ai quelques méthodes qui effectuent certaines modifications de données dans une base de données (insérer, mettre à jour et supprimer). L' ORM que j'utilise renvoie des valeurs int affectées par les lignes pour ce type de méthode. Que dois-je retourner pour "ma méthode", afin d'indiquer l'état de réussite / d'échec de l'opération?

Considérez le code qui renvoie un int:

A.1

public int myLowerLevelMethod(int id) {
    ...
    int affectedRows = myOrm.deleteById(id)
    ...

    return affectedRows;
}

Puis utilisation:

A.2

public void myOtherMethod() {
    ...
    int affectedRows = myLowerLevelMethod(id)

    if(affectedRows > 0) {
        // Success
    } else {
        // Fail
    }
}

Comparez avec l'utilisation de booléen:

B.1

public boolean myLowerLevelMethod(int id) {
    ...
    int affectedRows = myOrm.deleteById(id)
    ...

    return affectedRows > 0;
}

Puis utilisation:

B.2

public void myOtherMethod() {
    ...
    boolean isSuccess = myLowerLevelMethod(id)

    if(isSuccess) {
        // Success
    } else {
        // Fail
    }
}

Lequel (A ou B) est le meilleur? Ou les avantages / inconvénients de chacun?

Hoang Tran
la source
Dans votre "A.2". Si zéro ligne est affectée, pourquoi est-ce un échec si zéro ligne doit être affectée? En d'autres termes, s'il n'y a pas d'erreur de base de données, pourquoi est-ce un échec?
Jaydee
5
Existe-t-il une différence sémantique entre «échec» et «zéro ligne affectée»? Par exemple, lors de la suppression de toutes les commandes d'un client, il y a une différence entre "le client n'existe pas" et "le client n'a pas de commandes".
Cephalopod
Considérez-vous une suppression avec zéro ligne inattendue? Dans ce cas, jetez.
usr
@Arian Je pense que c'est la vraie question pour moi. Je pense que j'ai choisi B, car avec A, mon code contient maintenant la vérification de 0 dans certains endroits et de -1 dans d'autres
Hoang Tran

Réponses:

18

Une autre option consiste à renvoyer un objet résultat au lieu des types de base. Par exemple:

OperationResult deleteResult = myOrm.deleteById(id);

if (deleteResult.isSuccess()) {
    // ....
}

Avec cela, si pour une raison quelconque vous devez renvoyer le nombre de lignes affectées, vous pouvez simplement ajouter une méthode dans OperationResult:

if (deleteResult.isSuccess()) {
    System.out.println("rows deleted: " + deleteResult.rowsAffected() );
}

Cette conception permet à votre système de croître et d'inclure de nouvelles fonctionnalités (connaître les lignes affectées) sans modifier le code existant.

AlfredoCasado
la source
2
+1 "Cette conception permet à votre système de se développer et d'inclure de nouvelles fonctionnalités (connaître les lignes affectées) sans modifier le code existant" C'est la bonne façon de penser à cette catégorie de questions.
InformedA
J'ai fait la même chose dans mon ancien projet. J'ai un objet de requête et de résultat wrapper. Les deux utilisent des compositions pour inclure plus de détails. et les deux contiennent également des données de base. Dans ce cas, l'objet Result a un code d'état et un champ msg.
InformedA
Surtout pour un système utilisant un ORM, j'appellerais cette meilleure pratique. L'ORM en question peut également déjà inclure ce type d'objet de résultat!
Brian S
En regardant l'exemple des OP, tout ce à quoi je peux penser, c'est que deleteById retournera 2 à un moment donné :) donc définitivement un type personnalisé, oui.
deadsven
J'aime cette approche. Je pense qu'il est non bloquant, couvre mes deux approches et modifiable / extensible (à l'avenir, si nécessaire). Le seul inconvénient auquel je peux penser est que c'est un peu plus de code, surtout quand il arrive au milieu de mon projet. Je vais marquer cela comme la réponse. Je vous remercie.
Hoang Tran
12

Renvoyer le nombre de lignes affectées est préférable car il fournit des informations supplémentaires sur le déroulement de l'opération.

Aucun programmeur ne vous en voudra car il / elle doit écrire ceci pour vérifier s'ils ont obtenu des changements pendant l'opération:

if(affectedRows > 0) {
    // success
} else {
    // fail
}

mais ils vous blâmeront lorsqu'ils devront connaître le nombre de lignes affectées et ils se rendront compte qu'il n'y a pas de méthode pour obtenir ce nombre.

BTW: si par "échec" vous voulez dire une erreur de requête syntaxique (auquel cas le nombre de lignes affectées est évidemment 0), alors lever l'exception serait plus approprié.

Policier
la source
4
-1 pour la raison que vous avez donnée. Donner des informations supplémentaires n'est pas toujours une bonne idée. Souvent, une meilleure conception conduirait à communiquer à l'appelant exactement ce dont il a besoin, et rien de plus.
Arseni Mourzenko
1
@MainMa rien ne s'oppose à la création de méthodes surchargées. De plus, dans les langues natives (famille C par exemple), le nombre de lignes peut être utilisé directement dans la logique (0 est faux, tout le reste est vrai).
PTwr
@PTwr: donc pour faire face au fait que la méthode retourne trop d'informations, vous proposez de créer une surcharge? Cela ne semble pas correct. Quant à "zéro est faux, non nul est vrai", ce n'est pas l'objet de mon commentaire, et c'est une mauvaise pratique dans certaines langues.
Arseni Mourzenko
1
Je suis probablement l'un des programmeurs gâtés car je ne pense pas que l'utilisation de trucs puisse être considérée comme une bonne pratique. En fait, chaque fois que je dois gérer le code de quelqu'un d'autre, je suis reconnaissant à celui qui l'a écrit et n'a pas utilisé d'astuces.
proskor
4
Vous devez toujours renvoyer le nombre de lignes affectées ici !!! Puisque vous ne pouvez pas savoir si le nombre de lignes = 0 est un échec, ou si le nombre de lignes = 3 est un succès? Si quelqu'un veut insérer 3 lignes et seulement 2 sont insérées, vous retournerez vrai, mais ce n'est pas vrai! Et si quelqu'un veut update t set category = 'default' where category IS NULLégaliser 0 lignes affectées serait un succès, car maintenant il n'y a pas d'élément sans catégorie, même si aucune ligne n'a été affectée!
Falco
10

Je ne recommanderais aucun d'entre eux. Au lieu de cela, ne retournez rien (vide) en cas de succès et lancez une exception en cas d'échec.

C'est exactement pour la même raison que je choisis de déclarer certains membres de la classe privés. Il rend également la fonction plus facile à utiliser. Un contexte plus opérationnel ne signifie pas toujours mieux, mais cela signifie certainement plus complexe. Moins vous promettez, plus vous éloignez, plus il est facile pour le client de comprendre et plus vous avez de liberté pour choisir comment le mettre en œuvre.

La question est de savoir comment indiquer le succès / l'erreur. Dans ce cas, il suffit de signaler l'échec en lançant une exception et de ne rien retourner en cas de succès. Pourquoi dois-je fournir plus que les besoins des utilisateurs?

Des échecs / situations exceptionnelles peuvent survenir et vous devrez alors y faire face. Que vous utilisiez try / catch pour le faire ou que vous examiniez les codes de retour, c'est une question de style / préférence personnelle. Les idées derrière try / catch sont les suivantes: séparer le flux normal du flux exceptionnel et laisser les exceptions remonter jusqu'à la couche où elles peuvent être gérées de la manière la plus appropriée. Ainsi, comme beaucoup l'ont déjà souligné, cela dépend si l'échec est vraiment exceptionnel ou non.

proskor
la source
4
-1 Pourquoi ne retourneriez-vous rien où vous pourriez retourner quelque chose, sans effets secondaires négatifs, qui fournit un contexte opérationnel supplémentaire?
FreeAsInBeer
7
Pour exactement la même raison, j'ai choisi de déclarer certains membres de la classe privés. Il rend également la fonction plus facile à utiliser. Un contexte plus opérationnel ne signifie pas toujours mieux, mais cela signifie certainement plus complexe. Moins vous promettez, plus vous éloignez, plus il est facile pour le client de comprendre et plus vous avez de liberté pour choisir comment le mettre en œuvre.
proskor
1
Eh bien, quelles données l'utilisateur a-t-il réellement besoin d'obtenir? La question est de savoir comment indiquer le succès / l'erreur. Dans ce cas, il suffit de signaler l'échec en lançant une exception et de ne rien retourner en cas de succès. Pourquoi dois-je fournir plus que les besoins des utilisateurs?
proskor
4
@proskor: Les exceptions concernent des cas exceptionnels. «Échec» dans ce scénario peut être un résultat attendu. Bien sûr, présentez-le comme une alternative potentielle, mais il n'y a pas assez d'informations ici pour faire une recommandation.
Nick Barnes
1
-1 Les exceptions ne doivent pas faire partie du flux de programme normal. On ne sait pas ce que signifie "échec" dans le contexte de votre erreur, mais une exception dans le contexte d'un appel à la base de données devrait être due à une exception qui se produit dans la base de données. L'affectation de zéro ligne ne doit pas être une exception. Une requête mutilée qui ne peut pas être analysée, fait référence à une table inexistante, etc. serait une exception car le moteur de base de données s'y étoufferait et le lancerait.
2

"Est-ce mieux que ça?" n'est pas une question utile lorsque les deux alternatives ne font pas la même chose.

Si vous avez besoin de connaître le nombre de lignes affectées, vous devez utiliser la version A. Si vous n'êtes pas obligé, vous pouvez utiliser la version B - mais tout avantage que vous pourriez gagner en termes d'effort d'écriture de code a déjà disparu depuis vous avez pris la peine de publier les deux versions sur un forum en ligne!

Mon point est le suivant: quelle solution est la meilleure dépend entièrement de vos besoins spécifiques pour cette application, et vous connaissez ces circonstances bien mieux que nous. Il n'y a pas de choix à l'échelle de l'industrie, sûr à utiliser, conforme aux meilleures pratiques, qui ne se fera pas virer dessus, qui est mieux en général ; vous devez y penser vous-même. Et pour une décision aussi facilement révisable que celle-ci, vous n'avez pas besoin de passer autant de temps à réfléchir non plus.

Kilian Foth
la source
Je suis d'accord que cela dépend beaucoup des exigences de l'application. Cependant, il semble que ce genre de situation n'est pas très unique, et je ne cherche pas une solution miracle mais simplement l'expérience d'autres personnes traitant de la même chose / d'une chose similaire (peut-être que la question est un peu trompeuse, j'aime les suggestions autres que A / B)
Hoang Tran
1

KISS et YAGNI sont deux des principes les plus importants dans la conception de logiciels maintenables .

  • KISS : Restez simple, stupide
  • YAGNI : Vous n'en aurez pas besoin

Ce n'est presque jamais une bonne idée de mettre dans une logique dont vous n'avez pas besoin immédiatement en ce moment . Parmi de nombreuses autres personnes, Jeff Atwood (un co-fondateur de StackExchange) a écrit à ce sujet , et d'après mon expérience, lui et d'autres partisans de ces concepts ont tout à fait raison.

Toute complexité que vous ajoutez à un programme a un coût, payé sur une longue période. Le programme devient plus difficile à lire, plus complexe à modifier et plus facile à détecter pour les bogues. Ne tombez pas dans le piège d'ajouter des choses "au cas où". C'est un faux sentiment de sécurité.

Il est rare que vous obteniez un bon code la première fois. Les changements sont inévitables; l'ajout d'une logique spéculative pour se préparer défensivement à des imprévus futurs ne vous protégera pas réellement d'avoir à refactoriser votre code lorsque l'avenir se révèle différent de ce que vous attendiez. L'entretien de la logique inutile / contingente est plus un problème de maintenabilité que la refactorisation ultérieure pour ajouter des fonctionnalités manquantes.

Ainsi, puisqu'il apparaît que tout ce dont votre programme a besoin pour l'instant est de savoir si l'opération a réussi ou échoué, la solution B que vous proposez (renvoyer un seul booléen) est la bonne approche. Vous pouvez toujours le refactoriser ultérieurement si l'exigence change. Cette solution est la plus simple et a la plus faible complexité (KISS) et fait exactement ce dont vous avez besoin et rien de plus (YAGNI).

Ben Lee
la source
-1

Les lignes entières ou un état d'erreur

Envisagez de renvoyer les lignes entières, au moins en tant qu'option d'exécution. Dans les insertions de base de données, vous devrez peut-être inspecter les données qui ont été insérées, car elles seront souvent différentes de celles que vous avez envoyées à la base de données; les exemples courants incluent les ID de ligne générés automatiquement (dont l'application aura probablement besoin immédiatement), les valeurs par défaut déterminées par la base de données et les résultats des déclencheurs si vous les utilisez.

D'un autre côté, si vous n'avez pas besoin des données renvoyées, vous n'avez pas non plus besoin du nombre de lignes affectées, car cela n'est pas utile pour le résultat de 0. S'il y a des erreurs, vous devez retourner quel type de une erreur s'est produite, d'une manière cohérente avec les principes de gestion des erreurs de votre projet (exceptions, codes d'erreur numériques, etc.); mais il existe des requêtes valides qui affecteront correctement 0 lignes (c'est-à-dire "supprimer toutes les commandes expirées" s'il n'y en a pas réellement).

Peter est
la source