Question

Semblable à Les littéraux codés en dur ont-ils jamais été acceptables? , mais je pense spécifiquement aux & "; chaînes magiques &"; ici.

Sur un grand projet, nous avons un tableau d'options de configuration comme celles-ci:

Name         Value
----         -----
FOO_ENABLED  Y
BAR_ENABLED  N
...

(Des centaines d’entre eux).

La pratique courante consiste à appeler une fonction générique pour tester une option comme celle-ci:

if (config_options.value('FOO_ENABLED') == 'Y') ...

(Bien entendu, il peut être nécessaire de cocher cette option à plusieurs endroits du code système.)

Lors de l’ajout d’une nouvelle option, j’envisageais d’ajouter une fonction permettant de masquer la & "chaîne magique &"; comme ceci:

if (config_options.foo_enabled()) ...

Cependant, mes collègues ont pensé que j’en faisais trop, préférant le codage en dur, car:

  • C'est ce que nous faisons normalement
  • Il est plus facile de voir ce qui se passe lors du débogage du code

Le problème, c’est que je vois ce qu’ils veulent dire! En réalité, nous n'allons jamais renommer les options pour quelque raison que ce soit. Le seul avantage que je puisse imaginer pour ma fonction est que le compilateur attraperait n'importe quelle faute de frappe comme fo_enabled (), mais pas 'FO_ENABLED'.

Qu'en penses-tu? Ai-je oublié d'autres avantages / inconvénients?

Était-ce utile?

La solution

if (config_options.isTrue('FOO_ENABLED')) {...
}

Limitez votre contrôle Y codé en dur à un seul emplacement, même si cela signifie écrire une classe wrapper pour votre carte.

if (config_options.isFooEnabled()) {...
}

Cela peut sembler correct tant que vous n’avez pas 100 options de configuration et 100 méthodes (vous pouvez donc ici juger de la croissance et des besoins futurs des applications avant de décider de votre implémentation). Sinon, il est préférable d’avoir une classe de chaînes statiques pour les noms de paramètres.

if (config_options.isTrue(ConfigKeys.FOO_ENABLED)) {...
}

Autres conseils

Si j'utilise une chaîne une fois dans le code, je ne m'inquiète généralement pas de la rendre constante quelque part.

Si j'utilise une chaîne deux fois dans le code, je considérerai en faire une constante.

Si j'utilise une chaîne trois fois dans le code, je vais certainement en faire une constante.

Je réalise que la question est ancienne, mais elle est apparue à ma marge.

AFAIC, la question ici n'a pas été identifiée avec précision, ni dans la question, ni dans les réponses. Oubliez les chaînes de harcodage & Quot; ou pas, pour un moment.

  1. La base de données possède une table de référence contenant config_options. La PK est une chaîne.

  2. Il existe deux types de PC:

    • Identificateurs significatifs, que les utilisateurs (et les développeurs) voient et utilisent. Ces PK sont supposées être stables, on peut s'y fier.

    • Des colonnes sans signification Id que les utilisateurs ne doivent jamais voir, que les développeurs doivent connaître et coder. Ceux-ci ne peuvent pas être invoqués.

  3. Il est ordinaire, normal d'écrire du code en utilisant la valeur absolue d'un PK significatif IF CustomerCode = "IBM" ... ou IF CountryCode = "AUS" etc.

    • référencer la valeur absolue d'une PK dénuée de sens n'est pas acceptable (en raison de l'incrémentation automatique; les écarts sont modifiés; les valeurs sont remplacées en gros).
      .
  4. Votre table de référence utilise des PK significatives. Référencer ces chaînes littérales dans le code est inévitable. Cacher la valeur rendra la maintenance plus difficile; le code n'est plus littéral; vos collègues ont raison. De plus, il y a la fonction redondante supplémentaire qui mâche les cycles. S'il y a une faute de frappe dans le littéral, vous le saurez bientôt lors des tests de développement, bien avant l'UAT.

    • des centaines de fonctions pour des centaines de littéraux est absurde. Si vous implémentez une fonction, normalisez votre code et fournissez une fonction unique pouvant être utilisée pour n'importe lequel des centaines de littéraux. Dans ce cas, nous sommes revenus à un littéral nu et la fonction peut être supprimée.

    • Le fait est que la tentative de masquer le littéral n'a aucune valeur.
      .

  5. Cela ne peut pas être interprété comme & "; codage en dur &"; c'est quelque chose de tout à fait différent. Je pense que c’est là que réside votre problème, en identifiant ces constructions comme & "; Codées en dur &"; Il ne fait que référencer littéralement une PK significative.

  6. Maintenant, du point de vue de tout segment de code uniquement, si vous utilisez la même valeur plusieurs fois, vous pouvez améliorer le code en capturant la chaîne littérale dans une variable, puis en utilisant la variable dans le reste de la liste. bloc de code. Certainement pas une fonction. Mais c’est un problème d’efficacité et de bonnes pratiques. Même cela ne change pas l'effet IF CountryCode = @cc_aus

D'après mon expérience, ce type de problème masque un problème plus profond: le fait de ne pas exécuter la POO réelle et de suivre le principe de DRY.

En résumé, capturez la décision au démarrage avec une définition appropriée pour chaque action dans , dans les instructions if, puis jetez les tests config_options et d'exécution.

Détails ci-dessous.

L'échantillon utilisé était:

if (config_options.value('FOO_ENABLED') == 'Y') ...

ce qui soulève la question évidente, & "Que se passe-t-il dans les points de suspension? &" ;, surtout compte tenu de la déclaration suivante:

  

(Bien entendu, il peut être nécessaire de cocher cette option à plusieurs endroits du code système.)

Supposons que chacune de ces config_option valeurs correspond vraiment à un seul concept de domaine de problème (ou de stratégie de mise en œuvre).

Au lieu de procéder ainsi (à plusieurs reprises, à différents endroits du code):

  1. Prenez une chaîne (balise),
  2. Recherchez son autre chaîne correspondante (valeur),
  3. Testez cette valeur en équivalent booléen,
  4. Sur la base de ce test, décidez si vous souhaitez effectuer une action.

Je suggère d'encapsuler le concept d'une " action configurable ".

Prenons un exemple (évidemment aussi hypthétique que FOO_ENABLED ... ;-) que votre code doit fonctionner en unités anglaises ou métriques. Si METRIC_ENABLED est & Quot; true & Quot ;, convertissez les données saisies par l'utilisateur de la métrique en anglais pour les calculs internes, puis reconvertissez-les avant d'afficher les résultats.

Définir une interface:

public interface MetricConverter {
    double toInches(double length);
    double toCentimeters(double length);
    double toPounds(double weight);
    double toKilograms(double weight);
}

qui identifie en un seul endroit tous les comportements associés au concept de metricOption().

Ensuite, écrivez des implémentations concrètes de toutes les manières dont ces comportements doivent être appliqués:

public class NullConv implements MetricConverter {
    double toInches(double length) {return length;}
    double toCentimeters(double length) {return length;}
    double toPounds(double weight)  {return weight;}
    double toKilograms(double weight)  {return weight;}
}

et

// lame implementation, just for illustration!!!!
public class MetricConv implements MetricConverter {
    public static final double LBS_PER_KG = 2.2D;
    public static final double CM_PER_IN = 2.54D
    double toInches(double length) {return length * CM_PER_IN;}
    double toCentimeters(double length) {return length / CM_PER_IN;}
    double toPounds(double weight)  {return weight * LBS_PER_KG;}
    double toKilograms(double weight)  {return weight / LBS_PER_KG;}
}

Au moment du démarrage, au lieu de charger un ensemble de <=> valeurs, initialisez un ensemble d'actions configurables, comme dans:

MetricConverter converter = (metricOption()) ? new MetricConv() : new NullConv();

(où l'expression <=> ci-dessus est une substitution pour toute vérification ponctuelle que vous devez effectuer, y compris en examinant la valeur de METRIC_ENABLED; -)

Ensuite, partout où le code aurait dit:

double length = getLengthFromGui();
if (config_options.value('METRIC_ENABLED') == 'Y') {
    length = length / 2.54D;
}
// do some computation to produce result
// ...
if (config_options.value('METRIC_ENABLED') == 'Y') {
    result = result * 2.54D;
}
displayResultingLengthOnGui(result);

réécrivez-le en tant que:

double length = converter.toInches(getLengthFromGui());
// do some computation to produce result
// ...
displayResultingLengthOnGui(converter.toCentimeters(result));

Étant donné que tous les détails de la mise en œuvre liés à ce concept sont désormais empaquetés proprement, toute maintenance future liée à <=> peut être effectuée à un endroit unique. De plus, le compromis au moment de l'exécution est une victoire; le " overhead " L’appel d’une méthode est trivial par rapport à la surcharge de récupérer une valeur de chaîne depuis une mappe et d’effectuer String # est égal à.

Je pense que les deux raisons que vous avez mentionnées, une faute d'orthographe possible dans string, ne peuvent être détectées qu'au moment de l'exécution et que la possibilité (bien que mince) d'un changement de nom justifie votre idée.

En plus de cela, vous pouvez obtenir des fonctions typées. Maintenant, il semble que vous ne stockiez que des booléens. Que se passe-t-il si vous devez stocker un int, une chaîne, etc. Je préférerais utiliser get_foo () avec un type, plutôt que get_string ( "FOO &"; ou get_int (& "FOO &";).

Je devrais vraiment utiliser des constantes et pas de littéraux codés en dur.

Vous pouvez dire qu'ils ne seront pas modifiés, mais vous ne le saurez peut-être jamais. Et il vaut mieux en faire une habitude. Utiliser des constantes symboliques.

Je pense qu'il y a deux problèmes différents ici:

  • Dans le projet en cours, la convention d'utilisation de chaînes codées en dur est déjà bien établie. Elle est donc familière à tous les développeurs travaillant sur le projet. Ce pourrait être une convention sous-optimale pour toutes les raisons énumérées, mais toute personne familiarisée avec le code peut le consulter et sait instinctivement ce que le code est censé faire. Changer le code de sorte que dans certaines parties, il utilise le & "Nouveau &"; la fonctionnalité rendra le code un peu plus difficile à lire (car les utilisateurs devront réfléchir et se rappeler ce que fait la nouvelle convention) et donc un peu plus difficile à maintenir. Mais j’imagine que le passage de l’ensemble du projet à la nouvelle convention aurait un coût prohibitif, à moins que vous ne puissiez scripter rapidement la conversion.
  • Sur un nouveau projet, les constantes symboliques sont le moyen utilisé par l’OMI pour toutes les raisons énumérées. Surtout , car tout ce qui rend le compilateur intercepté des erreurs au moment de la compilation qui seraient autrement interceptées par un humain au moment de l'exécution est une convention très utile à établir.

Moi aussi, je préfère une classe de configuration fortement typée si elle est utilisée dans le code. Avec des méthodes bien nommées, vous ne perdez aucune lisibilité. Si vous devez effectuer des conversions de chaînes en un autre type de données (décimal / float / int), vous n'avez pas besoin de répéter le code qui effectue la conversion à plusieurs endroits et pouvez mettre en cache le résultat afin que la conversion ne soit effectuée qu'une seule fois. Vous avez déjà les bases de cette base déjà en place, donc je ne pense pas qu'il en faudrait beaucoup pour vous habituer à la nouvelle façon de faire les choses.

Une autre chose à considérer est l'intention. Si vous êtes sur un projet qui nécessite une localisation, les chaînes codées en dur peuvent être ambiguës. Considérez ce qui suit:

const string HELLO_WORLD = "Hello world!";
print(HELLO_WORLD);

L'intention du programmeur est claire. L'utilisation d'une constante implique que cette chaîne n'a pas besoin d'être localisée. Maintenant, regardez cet exemple:

print("Hello world!");

Ici, nous ne sommes pas si sûrs. Le programmeur ne souhaitait-il vraiment pas que cette chaîne soit localisée ou avait-il oublié la localisation pendant qu'il écrivait ce code?

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top