patternjavaMinor
Union of intervals
Viewed 0 times
unionintervalsstackoverflow
Problem
I am trying to solve for the problem with a bunch of intervals:
I find their union, which in the above given case would be:
It will be great to get some feedback on whether I am on the right track and how I can improve upon what I have.
```
import java.util.Collections;
import java.util.List;
import java.util.ArrayList;
public class IntervalUnion{
class Interval implements Comparable{
private final int left;
private final int right;
Interval(int left,int right){
this.left = left;
this.right = right;
}
public int getRight(){
return this.right;
}
public int getLeft(){
return this.left;
}
@Override
public int compareTo(Interval interval){
if(this.left > interval.left){
return 1;
}else if(this.left getUnion(List intervals){
Collections.sort(intervals);
List union = new ArrayList();
int left = Integer.MAX_VALUE;
int right = Integer.MIN_VALUE;
for(Interval i:intervals){
int currentRight = i.getRight();
int currentLeft = i.getLeft();
if(currentLeft right){
right = currentRight;
}
if(currentLeft = right){
left = currentLeft;
right = currentRight;
}
if(currentLeft > right){
union.add(new Interval(left,right));
left = currentLeft;
right = currentRight;
}
}
union.add(new Interval(left,right));
return union;
}
public static void main(String[] args){
System.out.println("Testing out the union");
IntervalUnion intervalUnion = new IntervalUnion();
IntervalUnion.Interval i1 = intervalUnion.new Interval(2,4);
IntervalUnion.Interval i2 = intervalUnion.new Interval(1,1);
IntervalU
(1,2),(2,5),(4,7),(9,11)I find their union, which in the above given case would be:
(1,7),(9,11)It will be great to get some feedback on whether I am on the right track and how I can improve upon what I have.
```
import java.util.Collections;
import java.util.List;
import java.util.ArrayList;
public class IntervalUnion{
class Interval implements Comparable{
private final int left;
private final int right;
Interval(int left,int right){
this.left = left;
this.right = right;
}
public int getRight(){
return this.right;
}
public int getLeft(){
return this.left;
}
@Override
public int compareTo(Interval interval){
if(this.left > interval.left){
return 1;
}else if(this.left getUnion(List intervals){
Collections.sort(intervals);
List union = new ArrayList();
int left = Integer.MAX_VALUE;
int right = Integer.MIN_VALUE;
for(Interval i:intervals){
int currentRight = i.getRight();
int currentLeft = i.getLeft();
if(currentLeft right){
right = currentRight;
}
if(currentLeft = right){
left = currentLeft;
right = currentRight;
}
if(currentLeft > right){
union.add(new Interval(left,right));
left = currentLeft;
right = currentRight;
}
}
union.add(new Interval(left,right));
return union;
}
public static void main(String[] args){
System.out.println("Testing out the union");
IntervalUnion intervalUnion = new IntervalUnion();
IntervalUnion.Interval i1 = intervalUnion.new Interval(2,4);
IntervalUnion.Interval i2 = intervalUnion.new Interval(1,1);
IntervalU
Solution
No comments about the logic (yet?), but I do have something else to comment...
Validation
You should validate that the
Endpoints
Slightly pernickety on this, but since your intervals includes both endpoints, your
If you make
Comparisons
Your
Creating
Using
Proper unit testing, please
Just displaying the output is not a unit test, you should assert that the output of calling
What's good
I like that your
Other suggestions
I wonder if this is a good case for Stream reduction operations...
edit: I now have my own take on your question/approach here, using stream reduction:
Onion Run Festival (or the unexpected anagram for Union of Intervals)
Validation
You should validate that the
left field is never greater than the right field during instantiation, since your comparison logic makes use of that.Endpoints
Slightly pernickety on this, but since your intervals includes both endpoints, your
toString() representation should be using square braces instead of parentheses... See here for more info.static classes?If you make
Interval a static class, you can 'freely' create instances without bounding to an instance of IntervalUnion. This really depends on your use case...Comparisons
Your
compareTo(Interval) code can be replaced by Integer.compareTo(int, int).Creating
ListsUsing
Arrays.asList() is probably what you want, instead of the double-brace initialization...Proper unit testing, please
Just displaying the output is not a unit test, you should assert that the output of calling
intervalUnion.getUnion(sample) matches an expected Collection of Interval objects. For more info: Unit testing = Arrange, Act and Assert (link to another good CR answer).What's good
I like that your
Interval class is immutable.Other suggestions
I wonder if this is a good case for Stream reduction operations...
edit: I now have my own take on your question/approach here, using stream reduction:
Onion Run Festival (or the unexpected anagram for Union of Intervals)
Context
StackExchange Code Review Q#85044, answer score: 5
Revisions (0)
No revisions yet.