Skip to main content
deleted 1 character in body
Source Link
Jeroen Vannevel
  • 11.6k
  • 2
  • 39
  • 79

I would suggest using a named parameterargument for readability:

I would suggest using a named parameter for readability:

I would suggest using a named argument for readability:

deleted 24 characters in body
Source Link
Jeroen Vannevel
  • 11.6k
  • 2
  • 39
  • 79
public static bool InOrderEqual(bool equalOk, params int[] values) { intfor(var previousValuei = values[0]; foreach(var1; valuei in< values.Skip(1)Length; i++) { if(!(valuevalues[i] > previousValuevalues[i - 1] || (valuevalues[i] >= previousValuevalues[i -1] && equalOk))) { return false; }  } return true; } 
public static bool InOrderEqual(bool equalOk, params int[] values) { int previousValue = values[0]; foreach(var value in values.Skip(1)) { if(!(value > previousValue || (value >= previousValue && equalOk))) { return false; }  } return true; } 
public static bool InOrderEqual(bool equalOk, params int[] values) { for(var i = 1; i < values.Length; i++) { if(!(values[i] > values[i - 1] || (values[i] >= values[i -1] && equalOk))) { return false; } } return true; } 
Source Link
Jeroen Vannevel
  • 11.6k
  • 2
  • 39
  • 79

InOrderEqual() 

Methods should express an action or a state, this doesn't do that. How about.. VerifyOrder? Or IsInOrder?


InOrderEqual(2, 5 , 11, false) 

I would suggest using a named parameter for readability:

InOrderEqual(2, 5 , 11, equalOk: false) 

I don't like "ok", it seems indecisive. I'd change that to "equalityAllowed".


You never use isEqual.


Your code is buggy. Try InOrderEqual(5, 5, 3, equalOk: true) and see what it returns.

This is because of this rather senseless statement:

if (b == a || (a == b && a == c)) 

If b == a and equalOk is set to true, suddenly you assume the third argument is in order as well.


if (equalOk == true) 

shortened as

if (equalOk) 

Console.WriteLine(true); 

Do not print to the console from your domain, leave that to whoever is calling your method. Now you're printing and returning data.


else Console.WriteLine(false); return false; 

Nasty! This might not be doing what you think it is. Translated to proper code, this says

else { Console.WriteLine(false); } return false; 

Notice how the return statement falls outside the else block? Always use parenthesis to properly denote a block and avoid confusion/logical errors.


else if (( a + a) - b == (b + b) - c|| b - a == c - b && c > b && b > a) 

This could use some commentary behind its approach and some examples. I shouldn't have to parse this entire expression in my head just to know what you're doing. Perhaps split it up in intermediate variables? Properly name the resulting variables and that would help a lot as well.


Your code is not extensible to more (or less) than 3 parameters even though this shouldn't be so hard.

Below is an example how I would do it with 3 hardcoded parameters:

public static bool InOrderEqual(int a, int b, int c, bool equalOk) { bool firstAndSecondInOrder = (a < b || (equalOk && a <= b)); bool secondAndThirdInOrder = (b < c || (equalOk && b <= c)); return firstAndSecondInOrder && secondAndThirdInOrder; } 

Or with multiple values:

public static bool InOrderEqual(bool equalOk, params int[] values) { int previousValue = values[0]; foreach(var value in values.Skip(1)) { if(!(value > previousValue || (value >= previousValue && equalOk))) { return false; } } return true; }