patterncsharpMinor
Summing up distinct elements in steps
Viewed 0 times
summingdistinctstepselements
Problem
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.
Another condition of the task is that LINQ cannot be used, as well as any of the
I have working code the solves the problem but the task requires multiple methods/functions. I have been trying to restructure the program but with all sorts of issues.
I have placed arrows in the code above to show where my issue is. If I try to declare "high" and "low" as global variables, my final answer is always a few numbers off, buy if I leave the variables declared as
What I want ideally is to have separate methods for each step of the calculation, so right now I have method for finding the number of times a specific value shows up in the
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 (you can see by my previously ask questions that has been a bit of a pain for me, since I solved this with LINQ in less than 5 minutes).I have working code the solves the problem but the task requires multiple methods/functions. I have been trying to restructure the program but with all sorts of issues.
using System;
using System.Collections.Generic;
//using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace Scoring {
class Program {
static int highOccurrence = 0;
static int lowOccurrence = 0;
//static int high; scores[i]) {
low = scores[i];
} //record lowest value
if (high 1) { //if there is more than 1 high (or 1 low) it is added once into the total
total += high;
if (lowOccurrence > 1) {
total += low;
}
}
Console.WriteLine("Sum = " + total);
return total; //remove not all code paths return.. error
}
static void ExitProgram() {
Console.Write("\n\nPress any key to exit program: ");
Console.ReadKey();
}//end ExitProgram
}
}I have placed arrows in the code above to show where my issue is. If I try to declare "high" and "low" as global variables, my final answer is always a few numbers off, buy if I leave the variables declared as
high = scores[0] etc, I will get the right answer.What I want ideally is to have separate methods for each step of the calculation, so right now I have method for finding the number of times a specific value shows up in the
Solution
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
So the programflow can be written as:
So you will have 8 methods without the
Review
Naming
Based on the naming conventions for C#, all methodnames should be written using the
So based on the list of tasks above the methods should be named
SetArrayitemsByValueToZero() which manipulates the items in the array
Style
Opening brackets should be placed on the line below the statement so
should be
Programflow
By combining some of these tasks you are violating the single responsible principle.
The
Refactoring
For getting the highest/lowest item in the array we can use the
Based of the comment as it is not allowed for this assignment to use Math.Min() or Math.Max() I have changed the methods
Next we should create a new calssed named
now we add a method to set each item of the array to zero if the value is either the max or the min of the array.
Next we will add a 2 overloaded
and last we add the missing
```
private BoundaryItem PreProcessArray(int[] items)
{
BoundaryItem boundaryItem = new BoundaryItem();
boundaryItem.Min = GetLowestItem(items);
boundaryItem.Max = GetHighestItem(items);
int minOccurance = GetCountOfOccurance(items, boundaryItem.Min);
int maxOccurance = GetCountOfOccurance(items, boundaryItem.Max);
SetArrayitemsByValueToZero(items, boundaryItem);
if (minOccurance < 2) { boundaryItem.Min = 0; }
if (maxOccurance < 2) { boundaryItem.Max = 0; }
return
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
So the programflow can be written as:
- Find the highest item of the array
- Find the lowest item of the array
- Find the number of occurance of the highest item of the array
- Find the number of occurance of the lowest item of the array
- Remove the first highest item found
- Remove the first lowest item found
- Sum together the remaining items
- Export/Print the results
So you will have 8 methods without the
ExitProgram() method. Review
Naming
Based on the naming conventions for C#, all methodnames should be written using the
PascalCasing style. So findOccurrence should be FindOccurrence and findScore should be FindScore So based on the list of tasks above the methods should be named
- FindHighestItem() or GetHighestItem()
- FindLowestItem() or GetLowestItem();
- FindNumberOfHighestOccurance() or GetCountOfOccurance()
- FindNumberOfLowestOccurance() or GetCountOfOccurance()
- RemoveHighestItems() or SetArrayitemsByValueToZero()
- RemoveLowestItems() or SetArrayitemsByValueToZero()
- GetHighestItemIndex()
- GetLowestItemIndex()
- GetNormalizedArray() which returns a new array or
SetArrayitemsByValueToZero() which manipulates the items in the array
- SumItems()
- Print()
Style
Opening brackets should be placed on the line below the statement so
for (int i = 0; i < scores.Length; i++) {should be
for (int i = 0; i < scores.Length; i++)
{Programflow
By combining some of these tasks you are violating the single responsible principle.
The
findScore() method - is searching for the highest and lowest item in the array
- removes the highest and lowest item from the array (basically add only the items which aren't highest or lowest to the new array)
- is summing the items of this new array stated above
- is printing/exporting the sum
Refactoring
For getting the highest/lowest item in the array we can use the
Math.Max() and Math.Min() methods. For finding the highest item we will initialize the var highestValue with Int32.MinValue and for finding the lowest item we will initialize the var lowestValue with Int32.MaxValue.Based of the comment as it is not allowed for this assignment to use Math.Min() or Math.Max() I have changed the methods
private int GetLowestItem(int[] items)
{
int lowestItem = Int32.MaxValue;
foreach (int item in items)
{
// lowestItem = Math.Min(item, lowestItem);
if (item highestItem)
{
highestItem = item;
}
}
return highestItem;
}private int GetCountOfOccurance(int[] items, int comparingValue)
{
int count = 0;
foreach (int item in items)
{
if (item == comparingValue)
{
count++;
}
}
return count;
}private int GetHighestItemIndex(int[] items, int highestItem)
{
for(int i = 0 ; i<items.Length ; i++)
{
if(items[i] == highestItem)
{
return i;
}
}
return -1;
}
private int GetLowestItemIndex(int[] items, int lowestItem)
{
// to be filled by you
}Next we should create a new calssed named
BoundaryItem which holds the min and max values of the array.class BoundaryItem
{
internal int Max { get; set; }
internal int Min { get; set; }
}now we add a method to set each item of the array to zero if the value is either the max or the min of the array.
private void SetArrayitemsByValueToZero(int[] items, BoundaryItem boundaryItem)
{
for (int i = 0; i < items.Length; i++)
{
if (items[i] == boundaryItem.Min || items[i] == boundaryItem.Max)
{
items[i] = 0;
}
}
}Next we will add a 2 overloaded
SumItems() methodspublic int SumItems(int[] items)
{
BoundaryItem boundaryItem = PreProcessArray(items);
return SumItems(items, boundaryItem);
}
private int SumItems(int[] items, BoundaryItem boundaryItem)
{
int sum = boundaryItem.Min + boundaryItem.Max;
foreach (int item in items)
{
sum += item;
}
return sum;
}and last we add the missing
PreProcessArray() method ```
private BoundaryItem PreProcessArray(int[] items)
{
BoundaryItem boundaryItem = new BoundaryItem();
boundaryItem.Min = GetLowestItem(items);
boundaryItem.Max = GetHighestItem(items);
int minOccurance = GetCountOfOccurance(items, boundaryItem.Min);
int maxOccurance = GetCountOfOccurance(items, boundaryItem.Max);
SetArrayitemsByValueToZero(items, boundaryItem);
if (minOccurance < 2) { boundaryItem.Min = 0; }
if (maxOccurance < 2) { boundaryItem.Max = 0; }
return
Code Snippets
for (int i = 0; i < scores.Length; i++) {for (int i = 0; i < scores.Length; i++)
{private int GetLowestItem(int[] items)
{
int lowestItem = Int32.MaxValue;
foreach (int item in items)
{
// lowestItem = Math.Min(item, lowestItem);
if (item < lowestItem)
{
lowestItem = item;
}
}
return lowestItem;
}
private int GetHighestItem(int[] items)
{
int highestItem = Int32.MinValue;
foreach (int item in items)
{
// highestItem = Math.Max(item, highestItem);
if (item > highestItem)
{
highestItem = item;
}
}
return highestItem;
}private int GetCountOfOccurance(int[] items, int comparingValue)
{
int count = 0;
foreach (int item in items)
{
if (item == comparingValue)
{
count++;
}
}
return count;
}private int GetHighestItemIndex(int[] items, int highestItem)
{
for(int i = 0 ; i<items.Length ; i++)
{
if(items[i] == highestItem)
{
return i;
}
}
return -1;
}
private int GetLowestItemIndex(int[] items, int lowestItem)
{
// to be filled by you
}Context
StackExchange Code Review Q#60975, answer score: 6
Revisions (0)
No revisions yet.