patternjavaMinor
Java Generics - Write a generic method to find the maximal element in the range [begin, end] of a list
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:
(imports omitted for brevity)
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 `
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:
With that method, for example, you could do:
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.