2
\$\begingroup\$

Task: You are given two rectangles.For each rectangle you are given its bottom-left and top-right points. Check if they overlap. If they do, check for bottom-left and top-right points of the overlapping area.

I decided to solve this by using multidimensional arrays. Using object might make it more transaprent tho. Is there anything, that I could change (especially in terms of task approach, code bugs, code style and performance ) ?

I came up with following code: Method wich checks if the give rectangles overlap

    public static bool Overlapping(int[][] firstRectangle, int[][] secondRectangle)
    {
        bool XOverlapping = secondRectangle[0][0] <= firstRectangle[1][0] && 
        firstRectangle[0][0] <= secondRectangle[1][0];
        bool YOverlapping = secondRectangle[0][1] <= firstRectangle[1][1] && 
        firstRectangle[0][1] <= secondRectangle[1][1];
        if (XOverlapping && YOverlapping) 
        {
            return true;
        }
        else
        {
            return false;
        }
    }

Method which checks for bottom-left and top-right points of the overlapping area.

 public static void GetOverlappingArea(int[][] firstRectangle, int[][] secondRectangle)
    {
        if (Overlapping(firstRectangle, secondRectangle))
        {
            int startXOverlapping = Math.Max(firstRectangle[0][0], secondRectangle[0][0]);
            int endXOverlapping = Math.Min(firstRectangle[1][0], secondRectangle[1][0]);
            int startYOverlapping = Math.Max(firstRectangle[0][1], secondRectangle[0][1]);
            int endYOverlapping = Math.Min(firstRectangle[1][1], secondRectangle[1][1]);
            Console.WriteLine("Left down corner: (" + startXOverlapping + " , " + startYOverlapping + "), Upper right corner: (" + endXOverlapping + " , " + endYOverlapping + ")");
        }
        else
        {
            Console.WriteLine("Rectangles do not overlap");
        }
    }

My test method

        static void Main(string[] args)
    { 
        int[][] FirstRectangle = new[] { new[] { 2, 4 }, new[] { 8, 7 } };
        int[][] SecondRectangle = new[] { new[] { 5, 7 }, new[] { 10, 15 } };
        GetOverlappingArea(FirstRectangle, SecondRectangle);
    }
\$\endgroup\$
5
\$\begingroup\$

Class selection

First, I believe a multidimensional array is not good enough for describing rectangles. For instance, it's easy to forget what dimension responsible for what direction:

int[][] FirstRectangle = new[] { new[] { 2, 4 }, new[] { 8, 7 } };

What exactly is 2, 4, 8 and 7?
You have to remember. It's a bad design.

Instead you could use the built-in Rectangle struct or reinvent the wheel - write your own type.
In the latter case, it makes sense to move processing methods into your type.


Naming

I personally think that a method name should describe what the method do and what it returns.

So my suggestion is to rename methods OverlappingIsOverlapping and GetOverlappingAreaPrintOverlappingArea.
Alternatively you could keep the name GetOverlappingArea (or Overlap, OverlapWith) and modify its code to create and return a new Rectangle instance which contains data of overlapping area. This method could be useful in the future.

Local variables should be named using camelCase: xOverlapping, yOverlapping.


Other notes

Instead of

if (XOverlapping && YOverlapping) 
{
    return true;
}
else
{
    return false;
}

You could just

return XOverlapping && YOverlapping;

Summarizing

public class Rectangle
{
    public readonly int Left;
    public readonly int Right;
    public readonly int Top;
    public readonly int Bottom;

    public Rectangle(int left, int right, int top, int bottom)
    {
        Left = left;
        Right = right;
        Top = top;
        Bottom = bottom;
    }

    public bool IsOverlapping(Rectangle other)
    {
        bool xOverlapping = other.Left <= Right && Left <= other.Right;
        bool yOverlapping = other.Top <= Bottom && Top <= other.Bottom;
        return xOverlapping && yOverlapping;
    }

    public Rectangle OverlapWith(Rectangle other)
    {
        if (!IsOverlapping(other))
        {
            return null;
        }

        int left = Math.Max(Left, other.Left);
        int right = Math.Min(Right, other.Right);
        int top = Math.Max(Top, other.Top);
        int bottom = Math.Min(Bottom, other.Bottom);

        return new Rectangle(left, right, top, bottom);
    }

    public override string ToString()
    {
        return $"Left={Left}, Right={Right}, Top={Top}, Bottom={Bottom}";
    }
}

TODO

What else should be done?

  • It always make sense to validate all input arguments in all public methods and constructors.
  • If you plan to compare rectangles with each other, it makes sense to implement IEquatable<Rectangle> interface and override the GetHashCode() method and optionally define == and != operators.
\$\endgroup\$
  • \$\begingroup\$ Thank you for your time and tips. Will keep them in mind. \$\endgroup\$ – Nashmár Mar 13 '18 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.