Skip to main content
Code Review

Return to Answer

deleted 4 characters in body
Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

It's good to have 2 methods. For the reason that @britishtea said in a comment, which is so spot-on I'm just going to quote it verbatim:

A method that takes a boolean value as a parameter and behaves differently depending on that value is not one method, but two methods! You should keep them separated. You couldc of coursec implement them using shared functionality, but your public API should offer two methods. :) – @britishtea

And indeed, this is how the .NET framework does it too (See StandardDeviation() and StandardDeviationP(), thanks @200_success).

At the same time, of course don't repeat yourself,. and extractExtract the common logic to a private helper method, and rewrite your public methods in terms of the private one:

private static double StandardDeviation(List<double> numberSet, double divisor)
{
 double mean = numberSet.Average(); 
 return Math.Sqrt(numberSet.Sum(x => Math.Pow(x - mean, 2)) / divisor);
}
static double PopulationStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count);
}
static double SampleStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count - 1);
}

Lastly, the parameter name "numberSet" is a misnomer, suggesting a Set, when it's really a List.

It's good to have 2 methods. For the reason that @britishtea said in a comment, which is so spot-on I'm just going to quote it verbatim:

A method that takes a boolean value as a parameter and behaves differently depending on that value is not one method, but two methods! You should keep them separated. You couldc of coursec implement them using shared functionality, but your public API should offer two methods. :) – @britishtea

And indeed, this is how the .NET framework does it too (See StandardDeviation() and StandardDeviationP(), thanks @200_success).

At the same time, of course don't repeat yourself, and extract the common logic to a private helper method, and rewrite your public methods in terms of the private one:

private static double StandardDeviation(List<double> numberSet, double divisor)
{
 double mean = numberSet.Average(); 
 return Math.Sqrt(numberSet.Sum(x => Math.Pow(x - mean, 2)) / divisor);
}
static double PopulationStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count);
}
static double SampleStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count - 1);
}

Lastly, the parameter name "numberSet" is a misnomer, suggesting a Set, when it's really a List.

It's good to have 2 methods. For the reason that @britishtea said in a comment, which is so spot-on I'm just going to quote it verbatim:

A method that takes a boolean value as a parameter and behaves differently depending on that value is not one method, but two methods! You should keep them separated. You couldc of coursec implement them using shared functionality, but your public API should offer two methods. :) – @britishtea

And indeed, this is how the .NET framework does it too (See StandardDeviation() and StandardDeviationP(), thanks @200_success).

At the same time, of course don't repeat yourself. Extract the common logic to a private helper method, and rewrite your public methods in terms of the private one:

private static double StandardDeviation(List<double> numberSet, double divisor)
{
 double mean = numberSet.Average(); 
 return Math.Sqrt(numberSet.Sum(x => Math.Pow(x - mean, 2)) / divisor);
}
static double PopulationStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count);
}
static double SampleStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count - 1);
}

Lastly, the parameter name "numberSet" is a misnomer, suggesting a Set, when it's really a List.

Source Link
janos
  • 112.9k
  • 15
  • 154
  • 396

It's good to have 2 methods. For the reason that @britishtea said in a comment, which is so spot-on I'm just going to quote it verbatim:

A method that takes a boolean value as a parameter and behaves differently depending on that value is not one method, but two methods! You should keep them separated. You couldc of coursec implement them using shared functionality, but your public API should offer two methods. :) – @britishtea

And indeed, this is how the .NET framework does it too (See StandardDeviation() and StandardDeviationP(), thanks @200_success).

At the same time, of course don't repeat yourself, and extract the common logic to a private helper method, and rewrite your public methods in terms of the private one:

private static double StandardDeviation(List<double> numberSet, double divisor)
{
 double mean = numberSet.Average(); 
 return Math.Sqrt(numberSet.Sum(x => Math.Pow(x - mean, 2)) / divisor);
}
static double PopulationStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count);
}
static double SampleStandardDeviation(List<double> numberSet)
{
 return StandardDeviation(numberSet, numberSet.Count - 1);
}

Lastly, the parameter name "numberSet" is a misnomer, suggesting a Set, when it's really a List.

lang-cs

AltStyle によって変換されたページ (->オリジナル) /