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

Java Generics - Write a generic method to find the maximal element in the range [begin, end] of a list

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

Problem

I'm a Java beginner, going through the Generics Questions and Exercises in Oracle's The Java Tutorials.

Here's my solution to #8:


Write a generic method to find the maximal element in the range [begin, end] of a list.

All suggestions for improvement are welcome. I'd especially like feedback on the following:

  • Java library usage



  • Algorithm performance



  • Exceptions



(imports omitted for brevity)

@SuppressWarnings("serial")
class MaximalElementList> extends ArrayList {

    /**
     * @param begin - list index to start iterating from (inclusive)
     * @param end - list index to iterate up to (exclusive)
     * @return max elem in list as determined by the compareTo of the element type.
     */
    public E getMaxInRange(int begin, int end) {
        E max;
        if (isEmpty())
            throw new IllegalStateException("Can't get a max element from an empty list.");
        else {
            max = this.get(begin);
            for (E elem : this.subList(begin, end)) {
                if (elem.compareTo(max) > 0) {
                    max = elem;
                }
            }
        }
        return max;
    }
}

Solution

So, here's the thing, what you have is not a generic method. What you have is a generic class.

Now, it's a clearly defined generic class, but, it is not what the question asked for. (as a generic class it has a number of issues too, but let's get the method/class issue resolved first).

Generic methods

A generic method is just that, a method, except the parameters (or return value) are of a generic type. A generic method always has a ` structure before the return-type declaration. A normal method is:

public RType methodName(P1Type p1, P2Type p2, ....) {...}


(where
RType is the return type, and P1Type is parameter1 type, etc.).

A generic method has the
before the return type, and that construct is used somewhere in the method signature... for example:

public  RType methodName(T t, SomeType sparm) {....}


The above is a generic method that has the generic type
T as a parameter.

Your Class

So, having stated that your solution is a generic class, not a generic method, let's assume the exercise goal was to produce a generic class. What then?

-
Unless you have exceptional reasons, don't extend
ArrayList, "compose" it instead. Have a class that does not extend ArrayList, and have a class field instead, like:

class MaximalElementList> {

    List data = ......


-
Your generics here are OK, no problem with
>

-
Your code should use a guard-condition instead of an else... let me explain. Your code has an if/else:

public E getMaxInRange(int begin, int end) {
    E max;
    if (isEmpty())
        throw new IllegalStateException("Can't get a max element from an empty list.");
    else {
        max = this.get(begin);
        for (E elem : this.subList(begin, end)) {
            if (elem.compareTo(max) > 0) {
                max = elem;
            }
        }
    }
    return max;
}


That should instead be:

public E getMaxInRange(int begin, int end) {
    if (isEmpty()) {
        throw new IllegalStateException("Can't get a max element from an empty list.");
    }

    E max;
    max = this.get(begin);
    for (E elem : this.subList(begin, end)) {
        if (elem.compareTo(max) > 0) {
            max = elem;
        }
    }
    return max;
}


-
Note now, that the
max is a messy variable, it can just be:

E max = this.get(begin);
    for (E elem : this.subList(begin, end)) {
        if (elem.compareTo(max) > 0) {
            max = elem;
        }
    }
    return max;


-
Your use of a
sublist` is smart, but I would consider it to be overkill in this case. How about a simpler implementation:

E max = this.get(begin);
    for (int i = begin + 1; i  0) {
            max = get(i);
        }
    }
    return max;


That is now some logic which I think would work well.

Making it a method

Putting that logic in a generic method would "simply" mean:

public > E max(List data, int begin, int end) {
    E max = data.get(begin);
    for (int i = begin + 1; i  0) {
            max = data.get(i);
        }
    }
    return max;

}


With that method, for example, you could do:

List data = Files.readAllLines(Paths.get("some file.txt"));
String maxLine = max(data, 0, 10);

Code Snippets

public RType methodName(P1Type p1, P2Type p2, ....) {...}
public <T> RType methodName(T t, SomeType sparm) {....}
class MaximalElementList<E extends Comparable<E>> {

    List<E> data = ......
public E getMaxInRange(int begin, int end) {
    E max;
    if (isEmpty())
        throw new IllegalStateException("Can't get a max element from an empty list.");
    else {
        max = this.get(begin);
        for (E elem : this.subList(begin, end)) {
            if (elem.compareTo(max) > 0) {
                max = elem;
            }
        }
    }
    return max;
}
public E getMaxInRange(int begin, int end) {
    if (isEmpty()) {
        throw new IllegalStateException("Can't get a max element from an empty list.");
    }

    E max;
    max = this.get(begin);
    for (E elem : this.subList(begin, end)) {
        if (elem.compareTo(max) > 0) {
            max = elem;
        }
    }
    return max;
}

Context

StackExchange Code Review Q#86210, answer score: 4

Revisions (0)

No revisions yet.