HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Calculating range of ArrayLists

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rangecalculatingarraylists

Problem

I have written this method which calculates the range (max - min) of an ArrayList. I have two for loops each for a different ArrayList. The for loops do exactly the same thing except they each use a different list, so I was wondering if there was a way to just use one for loop (or any other way) that does the work but iterates through both ArrayLists so that I don't have nearly identical for loops.

public static void calcRange(ArrayList  list1, ArrayList list2){
    int max = list1.get(0);
    int min = list1.get(0);
    int max2 = list2.get(0);
    int min2 = list2.get(0);

    for (int i : list1){
        if ( i > max){
            max = i;
        } else if (i  max2){
            max2 = i;
        } else if (i < min2){
            min2 = i;
        }
    }
    System.out.println("List 1 Range: "+(max - min)+"\tList 2 Range: "+(max2 - min2));
}

Solution

One way you can remove duplicated code is to create a method:

public static int[] GetMaxMin (ArrayList  list)
{
    int max = list.get(0);
    int min = list.get(0);

    for (int i : list){
        if (i > max) {
            max = i;
        } else if (i < min) {
            min = i;
        }
    }
    int[] maxMin = {max, min};

    return maxMin;
}


This can then be called like this:

max = GetMaxMin(list)[0];
min = GetMaxMin(list)[1];


However, the above said, I would not recommend doing this because it has two purposes - to get the max value and the min value in the list. What I would recommend is to implement two methods - one to get the max value and one to get the min value:

public static int GetMax (ArrayList  list)
{
    int max = list.get(0);

    for (int i : list){
        if (i > max) {
            max = i;
        }
    }

    return max;
}


GetMin would be similar. Then, all you would need to do is this:

int max = GetMax(list1);
int min = GetMin(list1);
int max2 = GetMax(list2);
int min2 = GetMin(list2);


As you are calculating a range of values, though, you should just write a method to do that:

public static int CalculateRange (ArrayList  list)
{
    int max = list.get(0);
    int min = list.get(0);

    for (int i : list){
        if (i > max) {
            max = i;
        } else if (i < min) {
            min = i;
        }
    }

    return max - min;
}


As brought up in the comments, this will crash if there are 0 elements in the list. To prevent this, you should probably check this before you try to get the range, like this:

if (list.size() != 0) {
    System.out.printf("List 1 Range: %d\tList 2 Range: %d\n", CalculateRange(list1), CalculateRange(list2));
} else {
    System.out.println("There are no elements in this array");
}

Code Snippets

public static int[] GetMaxMin (ArrayList <Integer> list)
{
    int max = list.get(0);
    int min = list.get(0);

    for (int i : list){
        if (i > max) {
            max = i;
        } else if (i < min) {
            min = i;
        }
    }
    int[] maxMin = {max, min};

    return maxMin;
}
max = GetMaxMin(list)[0];
min = GetMaxMin(list)[1];
public static int GetMax (ArrayList <Integer> list)
{
    int max = list.get(0);

    for (int i : list){
        if (i > max) {
            max = i;
        }
    }

    return max;
}
int max = GetMax(list1);
int min = GetMin(list1);
int max2 = GetMax(list2);
int min2 = GetMin(list2);
public static int CalculateRange (ArrayList <Integer> list)
{
    int max = list.get(0);
    int min = list.get(0);

    for (int i : list){
        if (i > max) {
            max = i;
        } else if (i < min) {
            min = i;
        }
    }

    return max - min;
}

Context

StackExchange Code Review Q#80393, answer score: 5

Revisions (0)

No revisions yet.