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

Rectangle Class

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

Problem

I'm trying to create a private graphics library.
Even tho I'm doing so for fun, I'd like to have some feedback about my coding.
For instance, what do you guys think of the following class (Rectangle)?
It does have some dependencies on other classes that I'm not including here just to save space.
If you guys think they're relevant, lemme know and I'll edit the answer to include'em.

Rectangle's dependencies are:

Point: A 2d point with integer coordinates.
Size: Two integers whos values can only be positive (maybe I should change it to uint?)
RectangleF: A 2d rectangle with float coordinates.


My most important questions are:

  • If I don't allow negative Widths nor Heights, should their types be uint? I'm using int because most of the "size" variables in .net are int (instead of uint). Ie: System.Collections.Generic.List's Count it a int, even tho it can never be negative.



  • Is it a bad design for my rectangle to implement IEnumerable? Is it counter intuitive?



Oh, and
I'm using public readonly fields (instead of get only properties) to make sure that even I can't change their values by accident.

```
namespace Trauer.Graphics
{
///
/// Immutable struct to replace mutable struct System.Drawing.Rectangle.
///
public struct Rectangle : IEquatable, IEnumerable
{
public readonly int X;
public readonly int Y;
public readonly int Width;
public readonly int Height;

public int Left => X;
public int Top => Y;
public int Right => X + Width;
public int Bottom => Y + Height;

public Point Location => new Point(X, Y);
public Size Size => new Size(Width, Height);

public static Rectangle Empty;

public Rectangle(int x, int y, int width, int height)
{
if (width new Rectangle(left, top, right - left, bottom - top);

public override string ToString() => $"{{X={X},Y={Y},Width={Width},Height={Height}}}";

public override in

Solution

Rectangle as IEnumerable

Is it a bad design for my rectangle to implement IEnumerable? Is it counter intuitive?

I find a Rectangle implementing the IEnumerable interface is weird and counter intuitive unless you have a good reason for implementing it. Unfortunatelly you didn't give any examples how to use it. Even the FromLTRB does not rely on this interface although it could be a good candidate but then you would need to store the coordinates in an array... or enumerate the values and check the indicies (perhaps with a switch). A lot of work.

Other then this I don't know what to expect from it and why would I need it.

Anyways, I think rectangle extensions would be better for this purpose because then you could write more then one that has a fixed order. In the name of the extension you could put the order of the coordinates e.g.:

public static IEnumerable AsEnumerableLTRB(this Rectangle rect) { ... }


or

public static IEnumerable AsEnumerableTRBL(this Rectangle rect) { ... }


alternatively only one extension with a PointOrder option.
implicit/explicit operators

public System.Drawing.Rectangle ToSystemRectangle() => new System.Drawing.Rectangle(X, Y, Width, Height);

public RectangleF ToRectangleF() => new RectangleF(X, Y, Width, Height);


These could do better as implicit or explicit operators.

I also wonder why you need a new rectangle? What can this one do better then the original one? The only difference I see is immutability but unless your graphics library can work directly with this rectangle I think the additional overhead of translating this type into the native one is an overkill and not worth it.

Int32 vs UInt32

If I don't allow negative Widths nor Heights, should their types be uint? I'm using int because most of the "size" variables in .net are int (instead of uint). Ie: System.Collections.Generic.List's Count it a int, even tho it can never be negative.

I think this question on Stack Overflow about [using uint vs int] can give you a good answer why you should use int and not uint. In short:

UInt32 is not CLS Compliant, which means that it is wholly inappropriate for use in public APIs. If you're going to be using uint in your private API, this will mean doing conversions to other types - and it's typically easier and safer to just keep the type the same.

(you should read the other answers on SO as well).

Code Snippets

public static IEnumerable<Point> AsEnumerableLTRB(this Rectangle rect) { ... }
public static IEnumerable<Point> AsEnumerableTRBL(this Rectangle rect) { ... }
public System.Drawing.Rectangle ToSystemRectangle() => new System.Drawing.Rectangle(X, Y, Width, Height);

public RectangleF ToRectangleF() => new RectangleF(X, Y, Width, Height);

Context

StackExchange Code Review Q#155707, answer score: 3

Revisions (0)

No revisions yet.