Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I looked at this question and thought that I could find a much simpler way of doing this, here is the criteria that was given:

My current task is to find a score from an array where the highest/lowest scores have been taken away, and if the highest/lowest occur more than once (ONLY if they occur more than once), one of them can be added:

E.g. int[] scores = [4, 8, 6, 4, 8, 5]

therefore the final addition will be \$\sum{4,8,6, 5} = 23 \$.

Another condition of the task is that LINQ cannot be used, as well as any of the System.Array methods

Here is what I came up with: (Link to Answer)

static void Main(string[] args)
{
    // take an array and sum the distinct numbers
    int[] numberArray = { 4, 8, 6, 4, 8, 5 };
    int[] numberArray2 = { 4, 4, 5, 6, 8, 8 };

    Console.WriteLine(sumSpecial(numberArray).ToString());
    Console.WriteLine(sumSpecial(numberArray).ToString());

    Console.ReadLine();
}

static int getHighestScore(int[] integerArray)
{
    var high = 0;
    foreach (int number in integerArray)
    {
        high = high < number ? number : high;
    }
    return high;
}

static int getLowestScore(int[] integerArray)
{
    var low = int.MaxValue;
    foreach (int number in integerArray)
    {
        low = low > number ? number : low;
    }
    return low;
}

static int sumWithoutHighAndLowScores(int[] integerarray)
{
    int sum = 0;
    int high = getHighestScore(integerarray);
    int low = getLowestScore(integerarray);
    foreach (int number in integerarray)
    {
        if (number != high && number != low)
        {
            sum += number;
        }
    }
    return sum;
}

//sum of numbers using high or low only if there is a duplicate of high or low
static int sumSpecial(int[] integerArray)
{
    var sum = sumWithoutHighAndLowScores(integerArray);
    var high = getHighestScore(integerArray);
    var low = getLowestScore(integerArray);

    var highs = 0;
    var lows = 0;
    foreach (int number in integerArray)
    {
        if (number == high) { highs++; }
        if (number == low) { lows++; }
    }
    if (lows > 1) { sum += low; }
    if (highs > 1) { sum += high; }

    return sum;
}

What could be done better or more efficiently without losing the intentions of the exercise?

share|improve this question
    
Why can't you use linq or System.Array? –  slartidan Aug 25 at 21:14
    
@slartidan, the Question I saw had those restrictions, so I followed suit. –  Malachi Aug 25 at 21:15

1 Answer 1

up vote 5 down vote accepted
  1. I think min and max are more descriptive names than high and low.

  2. In C# the standard naming convention for methods is PascalCase.

  3. You could save several iterations by doing the following things:

    • Create a single method for determining min and max in one go and return either a Tuple or a custom type or an array of length 2 or provide out parameters. If you don't want to use any more complex types than arrays then I'd probably go for out parameters (returning an array of a certain size doesn't seem very nice). So something along these lines:

      public void DetermineMinAndMax(int[] input, out int min, out int max)
      {
          min = Int32.Max;
          max = In32.Min;
          foreach (int number in input)
          {
              if (number < min) { min = number; }
              else if (number > max) { max = number; }
          }
      } 
      
    • Keep counters of how often you have seen the min/max already - if it's 1 add it otherwise skip. Something along these lines:

      public void SumSpecial(int[] input)
      {
          int min, max;
      
          DetermineMinAndMax(input, out min, out max);
      
          int countMinSeen = 0;
          int countMaxSeen = 0;
      
          int sum = 0;
      
          foreach (int number in input)
          {
              if (number == min)
              {
                  if (countMinSeen == 1) { sum += number; }
                  countMinSeen++;
              }
              else if (number == max)
              {
                  if (countMaxSeen == 1) { sum += number; }
                  countMaxSeen++;
              }
              else { sum += number; }
          }
      }
      

    This should reduce the number of required iterations to 2 instead of 4.

share|improve this answer
    
I have been mixed up all day today about the Method Naming, thank you for pointing that out. facepalm –  Malachi Aug 25 at 21:14

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.