Java object oriented code to find points lying in a rectangle

Question

I created this short code as an exercise to learn and practice my OOPS. The aim is - given a string of points as x1,y1,x2,y2... figure out how many of them lie inside a given rectangle.

Can anyone please point out what things I can improve in this code design and best practices wise.

public class Solution{

    private String points;

    public Solution(String points) {
        this.points = points;
    }

    @Override
    public String checkPointsInRectangle() {
        String[] numbers = points.split(", ");

        List<Point> pointList = getPointsList(numbers);

        SortedSet<Point> pointsByX =
                new TreeSet<>(Comparator.comparingInt((Point p) -> p.x)
                        .thenComparingInt((Point p) -> p.y));
        SortedSet<Point> pointsByY =
                new TreeSet<>(Comparator.comparingInt((Point p) -> p.y)
                        .thenComparingInt((Point p) -> p.x));

        pointsByX.addAll(pointList);
        pointsByY.addAll(pointList);

        Rectangle rect = new Rectangle(-3,7,-10,10);

        Set<Point> result = getPointsInRect(rect, pointsByX, pointsByY);

        StringBuilder joined = new StringBuilder();
        for(Point s : result){
            joined.append(s.x);
            joined.append(s.y);
        }
        return joined.toString();
    }

    List<Point> getPointsList(String [] points){
        List<Point> result = new ArrayList<>();
        for(int i=0;i< points.length-1;i++) {
            if (i % 2 == 0) {
                Point point = new Point(Integer.parseInt(points[i]), Integer.parseInt(points[i + 1]));
                result.add(point);
            }
        }
        return result;
    }

    Set<Point> getPointsInRect(Rectangle rect, SortedSet<Point> pointsByX, SortedSet<Point> pointsByY) {
        Point minXMinY = new Point(rect.x0, rect.y0);
        Point maxXMaxY = new Point(rect.x1, rect.y1);
        Set<Point> pointsX = pointsByX.subSet(minXMinY, maxXMaxY);
        Set<Point> pointsY = pointsByY.subSet(minXMinY, maxXMaxY);
        Set<Point> rectPoints = new HashSet<>(pointsY);
        rectPoints.retainAll(pointsX);
        return rectPoints;
    }
}

public class Point {

     final int x;       // made final so that user provided input is immutable.
     final int y;

    public Point(int x, int y){
        this.x = x;
        this.y = y;
    }
}

EDIT Added the Rectangle class as well

package dummy;

public class Rectangle {

    final int x0,x1,y0,y1;

    public Rectangle(int x0, int x1, int y0, int y1) {
        this.x0 = x0;
        this.x1 = x1;
        this.y0 = y0;
        this.y1 = y1;
    }
}

figure out how many of them lie inside a given rectangle is not true. You don't care about how many of them there are; you display all of them. — Reinderien
– Reinderien, Commented Aug 20, 2022 at 12:38
Umm... no? The getPointsInRect() function checks if points are in the given rectangle, then I add those to the rectPoints set — sweet_summer_child
– sweet_summer_child, Commented Aug 20, 2022 at 13:08
"How many of them" implies counting. You don't do any counting. — Reinderien
– Reinderien, Commented Aug 20, 2022 at 13:09
Did you perhaps mean given a string of points as x1,y1,x2,y2... figure out which of them lie on the boundary of a given rectangle ? — Reinderien
– Reinderien, Commented Aug 20, 2022 at 13:41

Reinderien · Accepted Answer · 2022-08-20 16:46:07Z

You're practicing OOP, but this doesn't make very good use of OOP patterns. Solution and checkPointsInRectangle incorrectly concern themselves with more than they should (set construction, comparison). Solution doesn't show an entry point and stores points as a member which it should not. Point should be responsible for its own parsing and formatting, and should be a record instead of a class. Rectangle should be responsible for point checking inside of its bounds.

Consider using the streaming interface, since this is really just an iterative parse, filter and format.

This:

for(Point s : result){
    joined.append(s.x);
    joined.append(s.y);
}

if intended for display purposes, wouldn't produce very useful output - there are no separators.

I'm trying to understand how your getPointsInRect could do what it's intended to do; I don't think it does. Set operations are not enough to calculate geometric containment unless you fill a set with every possible coordinate inside of the rect, which you don't - and even if you did, this would be pretty inefficient. Just do numeric bounds checking instead; no sets needed.

Suggested

Main.java

import java.util.stream.Collectors;

public class Main {
    public static void main(String[] args) {
        Rectangle rect = new Rectangle(-3,7, -10, 10);

        String contained = Point.parse("-11,-11,0,0,1,2")
            .filter(rect::contains)
            .map(Point::toString)
            .collect(Collectors.joining(", "));

        System.out.println(contained);
    }
}

Rectangle.java

public record Rectangle(int x0, int x1, int y0, int y1) {

    public boolean contains(Point p) {
        return x0 <= p.x() && p.x() <= x1
            && y0 <= p.y() && p.y() <= y1;
    }
}

Point.java

import java.util.stream.IntStream;
import java.util.stream.Stream;

public record Point(int x, int y) {
    public static Stream<Point> parse(String str) {
        String[] segments = str.split(",");

        return IntStream.range(0, segments.length/2)
            .mapToObj(i -> new Point(
                Integer.parseInt(segments[2*i]),
                Integer.parseInt(segments[2*i+1])
            ));
    }

    @Override
    public String toString() {
        return "(%d, %d)".formatted(x, y);
    }
}

Output

(0, 0), (1, 2)

Thankyou ! This was really helpful, learned a lot. Specially the use of java streams — sweet_summer_child
– sweet_summer_child, Commented Aug 21, 2022 at 2:26

Stack Exchange Network

Java object oriented code to find points lying in a rectangle

1 Answer 1

Suggested

Output

You must log in to answer this question.

Hot Network Questions

Java object oriented code to find points lying in a rectangle

1 Answer 1

Suggested

Output

You must log in to answer this question.

Related

Hot Network Questions