Original Code
public static bool IsPrimeNumber(int m)
{
bool yes = false;
int j = 2;
for (; j <= Math.Sqrt(m); j++)
{
if (m % j == 0)
{
return yes;
}
}
return yes = true;
}Refactoring Code
private bool IsPrimeNumber(int number)
{
for (int i = 2; i <= Math.Sqrt(number); i++)
{
if (number%i == 0) return false;
}
return true;
}Bemerkungen
- Public sollten nur wirklich die Funktionen sein die auch von außen Aufrufbar sein sollen. Das müssen Sie natürluch immer selbst entscheiden aber in diesem Beispiel gehen wir davon aus das die Funktion nur in dieser Klasse verwendet wurde. Falls sich das ändern sollte dann sollte auch unbeding eine Interface verwendet werden.
- "static" sollte nur in Ausnahmefällen verwendet werden wenn es unbedingt sein muss bzw. sinn macht. Wird meist aus Bequemlichkeit missbraucht und birgt Risiken oder Probleme die man eigentlich vermeiden will (Initialisierung oder UnitTests).
- Die Name des Parameters "m" ist wenig aufschlussreich. Klare eindeutige Bezeichnungen erhöhen das Verständnis, oder wissen sie was mit "m" gemeint ist? Den Zähler für die for-Schleife wurde ich ebenfalls auf den default Namen "i" umändern da es hier auch keinen bestimmten Grund gibt Ihn anders zu benennen.
- Warum die Zähler-Variable "j" außerhalt der for-Schleife deklariert wurde ist mir nicht bekannt macht aber hier überhaupt keinen Sinn.
- Die Vaiable "yes" ist hier ebenfalls überflüssig. Durch direktes return false oder true wird das gleiche verhalten erreicht und ist leichter zu verstehen.
- If-Abfragen in dieser Größen Ordnungen sind leserlicher wenn Sie direkt in einer Zeile geschrieben werden (ohne Geschweifte Klammern).
