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
{
/// <summary>
/// Immutable struct to replace mutable struct System.Drawing.Rectangle.
/// </summary>
public struct Rectangle : IEquatable<Rectangle>, IEnumerable<Point>
{
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 < 0)
throw new ArgumentOutOfRangeException(nameof(width) + " must be equal to or greater than zero.");
if (height < 0)
throw new ArgumentOutOfRangeException(nameof(height) + " must be equal to or greater than zero.");
X = x;
Y = y;
Width = width;
Height = height;
}
public Rectangle(Point location, Size size)
{
X = location.X;
Y = location.Y;
Width = size.Width;
Height = size.Height;
}
public static Rectangle FromLTRB(int left, int top, int right, int bottom) => new Rectangle(left, top, right - left, bottom - top);
public override string ToString() => $"{{X={X},Y={Y},Width={Width},Height={Height}}}";
public override int GetHashCode()
{
// Thanks Microsoft ._.
return unchecked((int)((uint)X ^
(((uint)Y << 13) | ((uint)Y >> 19)) ^
(((uint)Width << 26) | ((uint)Width >> 6)) ^
(((uint)Height << 7) | ((uint)Height >> 25))));
}
public override bool Equals(object obj) => obj is Rectangle && Equals((Rectangle)obj);
public bool Equals(Rectangle other)
{
return X == other.X &&
Y == other.Y &&
Width == other.Width &&
Height == other.Height;
}
public static bool operator ==(Rectangle left, Rectangle right)
{
return (left.X == right.X
&& left.Y == right.Y
&& left.Width == right.Width
&& left.Height == right.Height);
}
public static bool operator !=(Rectangle left, Rectangle right) => !(left == right);
public IEnumerator<Point> GetEnumerator()
{
return GetContainedPoints().GetGenericEnumerator();
}
IEnumerator IEnumerable.GetEnumerator()
{
return GetContainedPoints().GetEnumerator();
}
public Point[] GetContainedPoints()
{
var points = new Point[Width * Height];
int index = 0;
for (int y = Y; y < Bottom; y++)
{
for (int x = X; x < Right; x++)
{
points[index] = new Point(x, y);
index += 1;
}
}
return points;
}
public System.Drawing.Rectangle ToSystemRectangle() => new System.Drawing.Rectangle(X, Y, Width, Height);
public RectangleF ToRectangleF() => new RectangleF(X, Y, Width, Height);
public bool Contains(int x, int y)
{
return x >= X &&
x < Right &&
y >= Y &&
y < Bottom;
}
public bool Contains(Point pt) => Contains(pt.X, pt.Y);
public bool Contains(Rectangle other)
{
return other.X >= X &&
other.Right <= Right &&
other.Y >= Y &&
other.Bottom <= Bottom;
}
public Rectangle Intersect(Rectangle other)
{
int biggestX = Math.Max(X, other.X);
int smallestRight = Math.Min(Right, other.Right);
int biggestY = Math.Max(Y, other.Y);
int smallestBottom = Math.Min(Bottom, other.Bottom);
if (smallestRight >= biggestX && smallestBottom >= biggestY)
return new Rectangle(biggestX, biggestY, smallestRight - biggestX, smallestBottom - biggestY);
else
return Empty;
}
public bool IntersectsWith(Rectangle other)
{
return other.X < Right &&
X < other.Right &&
other.Y < Bottom &&
Y < other.Bottom;
}
public Rectangle Union(Rectangle other)
{
int smallestX = Math.Min(X, other.X);
int biggestRight = Math.Max(Right, other.Right);
int smallestY = Math.Min(Y, other.Y);
int biggestBottom = Math.Max(Bottom, other.Bottom);
return new Rectangle(smallestX, smallestY, biggestRight - smallestX, biggestBottom - smallestY);
}
}
}
Regarding a few questions:
Q: Why are you implementing your own graphics library? A: I started messing around with it when I was trying to understand why the system's Bitmap's Get/Set methods are so slow. Eventually I found myself having quite a lot of fun playing around with "my own library" (even if it wasn't suitable for production, since it was so poorly commented and had almost no unit tests), so I decided to continue playing with it.
Q: Why are you implementing your own Rectangle instead of using the System's one? A: First and foremost, because I don't like the idea of a mutating Rectangle. I'm not against mutable objects. But for "really simple things" like a struct that represent a Rectangle, I feel that changing one of its properties does change it's "essence". So I should create a new one instead. Second because I'm trying to reduce my library dependencies on System.Graphics: It doesn't feel right to create a graphics lib (that doesn't bring anything new to the table) that depends on another one (even bigger), right? So, even tho this is mostly a fun project, I'm trying to reduce my lib's dependencies on System.Graphics. Right now, my Bitmap class has a strong dependence on the System's one to read/write to files. And most of my classes contain a "ToSystemXXX" method. But in the future, if I implement a legit way to read / write a Bitmap to a files, I could just remove such methods and be "free" of the System.Graphics.
Q: Why does your rectangle implement IEnumerable? A: Well... That's one of the reasons I came here: to get feedback on that. My reasoning is that it makes other classes a little bit leaner. Bitmap, for instance, doesn't need to implement a "SetPixels(Rectangle, Color)" method, since it already implements a "SetPixels(IEnumerable, Color)". But I'm getting a "this is too pythonic for C# lib" feel. I'm not sure if this convenience justifies it's implementation, since it's hurting... I don't know... The "semantic" of the Rectangle.
Q: Did you steal Mycrosoft's GetHashCode implementation? A: Yes ._. as you can see in my GetHashCode comment.
Edit: I decided that my "shapes" won't implement IEnumerable. Even tho it's quite practical (and fun) to play with IEnumerables, I think it's not a good design choice, in this case. Mostly because it's not immediately clear which points you'll get when iterating of a shape that implements IEnumerable. In this case [rectangle], if you iterate over it, do you expect to get the points of its edge or the ones inside it? Or both? So yeah... I'll refactor my code. Thanks for the feedback guys and girls!
PointandSize? \$\endgroup\$