2017-05-14 1 views
1

J'ai ce code, que je crois est explicite (bien que laid):calcul du pourcentage Simplifier C#

public decimal Stat 
    { 
     get 
     { 
      if (Incorrect == 0) 
       return 100; 
      decimal x = (decimal)(Correct/Incorrect)/(decimal)(Correct + Incorrect) * 100; 
      return x; 
     } 
    } 

est-il un moyen de rendre ce code plus joli?

+0

Vous pouvez utiliser [?: Opérateur conditionnel] (https://docs.microsoft.com/en-us/dotnet/articles/csharp/language-reference/operators/conditional-operator), alias 'inline if' . – elgonzo

+0

Qu'essayez-vous de calculer? Le pourcentage correct? –

+0

Je pense que le code lisible est joli code. La seule chose que je peux penser pour le rendre plus lisible serait de mettre '(décimal) (Correct/Incorrect)' et '(décimal) (Correct + Incorrect)' dans des variables temporaires. Je trouve que moins de choses se passent sur une ligne pour être plus lisibles. Bien que si vous trouvez le code explicite, alors c'est lisible (et donc, je dirais, joli). –

Répondre

2
  1. Vous ne pouvez lancer qu'un seul côté de la division, ou simplement le multiplier par 100.0 au début (il ne sera plus nécessaire d'utiliser le format décimal).
  2. Vous ne devez pas prendre soin du boîtier de bord de == 0 incorrect, parce que si elle est vérifiée, alors 100 sera retourné (100,0 * X/(X + 0) est égal à 100,0)

    public decimal Stat 
    { 
        get 
        { 
         return 100m * Correct/(Correct + Incorrect); 
        } 
    } 
    
+0

Je suis également d'accord que l'opérateur ternaire rend le code un peu désordonné dans ce cas. – Pelle

+0

J'ai édité ma réponse; Je pense que le if-else n'est pas nécessaire ici. – Mockingbird

+0

Le résultat devrait toujours être 100 si Incorrect == 0, donc cela semble correct, si je ne me trompe pas. – Pelle

0

Cela devrait probablement:

public decimal Stat 
{ 
    get 
    { 
     decimal Total = (decimal)(Correct + Incorrect); 
     return (decimal)Correct/Total * 100.0M; 
    } 
} 
+1

Vous pouvez utiliser 100.0M au lieu de lancer. – Gusman

+0

pas de vérification si Incorrect == 0 – Marusyk

+1

@MegaTron La vérification 'Incorrect == 0' n'est pas nécessaire ... essayez-le. –

4

Vous devez utiliser decimal, pas int arithmétique. La façon la plus simple est de commencer avec la formule décimale 100m:

public decimal Stat 
{ 
    get 
    { 
     return (Correct + Incorrect == 0) 
      ? 100m 
      : 100m * Correct/(Correct + Incorrect); 
    } 
} 
+0

C'est une très bonne solution. J'aime ça! – Pelle

+0

Est-ce que je fais un bordel ici? la valeur '100m' devrait être à la fin droite? –

+0

@Luis Filipe: '100m' devrait être à l'extrémité * gauche *:' 100m' est de type 'decimal' c'est pourquoi' 100m * Correct', '100m * Correct/Incorrect' ... sont' decimal' et nous n'ont pas * problème de division entière *. –

1

Deux points,

  1. Un pourcentage correct serait Correct * 100m/(Correct + Incorrect). Vous avez divisé par incorrect à nouveau après cela. Je ne sais pas pourquoi c'est mais ça me semble faux.

  2. Le résultat de la division entière est un autre entier. Si Correct est 1 et Incorrect est 4 alors le résultat de Correct/Incorrect est 0. Toujours convertir aux types de virgule flottante avant de faire la division.

Je réécrivaient ce code comme si,

public int Total => Correct + Incorrect; 

// renamed "Stat" 
public decimal PercentageCorrect => (Correct * 100m)/Total; 

total semble être une quantité utile. Faisons juste une propriété. Renommer "Stat" rend évident ce que c'est. Juste en lisant votre code, j'ai dû demander ce que "Stat" était parce que ce n'était pas évident ce que vous essayiez de faire.

+0

Comme lire le code source Linux :) Je vois votre point, cependant. – Pelle