Open Chris3606 opened 4 years ago
A couple thoughts stand out.
Insert: if area is already sorted, I see no reason to have this function (unless they're part of the dressing involved with IEnumerable).
I would like to propose some operators if I might: + and += to add all points together, - and -= to remove all overlapping points, & to return all overlapping points, ^ to return all points in one or the other but not both, | to return all points of both.
Insert: if area is already sorted, I see no reason to have this function
I would contest that this may apply to many use cases, but not all. The assumption that inserting an element implies a strict sortable order may not be valid; you may, for example, just want to insert elements at the beginning to create something like a queue. Also, there are different efficiency implications to sorting a list via calling Sort
one or more times vs inserting elements in sorted order to begin with - in a use case where maximum performance is required, each of those options may be appropriate in some cases. List provides both of these methods and they have disparate use cases there, so my thinking is they may have disparate use cases here as well.
I would like to propose some operators if I might
The set operators are an interesting idea. There are already member and static functions that provide these operations currently, however the operators may be a convenient addition.
One caveat to the operators though is returning a new Area is not as efficient as modifying an existing one: for example, area -= area2
results in the allocation of a new Area, whereas area.RemoveAll(area2)
does not, so area.RemoveAll
may be measurably more performant.
Currently,
Area
works as an ordered list that is guaranteed to have distinct elements. Elements are returned in the order they are inserted viaPosition
, whilePositionsSet
returns aHashSet<Point>
of the points.Area
also provides functions likeContains(Point)
. Finally, it implementsIEnumerable<Point>
. I feel this has a number of shortcomings.Area
with either theArea.Positions
property, orArea
's implementation ofIEnumerable
.Point
is contained within the area with eitherArea.Contains(Point)
orArea.PositionsSet.Contains(Point)
myArea[12] = new Point(1, 2);
Insert
andSort
are missingBinarySearch
that could be defined inIReadOnlyList
are notProposal
I propose the following enhancements to remedy this difference and make
Area
more like what it claims to be: a list of points that cannot have duplicates and provides the fastest operations possible, while also providing some metadata about bounds.Additions
public Point this[int index] { get; set; }
to allow getting/setting thePoint
at the given indexBinarySearch
functions with overloads comparable toList<T>.BinarySearch
public void Clear()
method to clear all Pointspublic Point Find(Func<Point, bool> predicate)
to get first item matching predicate, or Point.None if no item existsFindIndex
overloads that matchList<T>.FindIndex
public Point FindLast(Func<Point, bool> predicate)
to get last item matching predicate, or Point.None if no item existsFindLastIndex
overloads that matchList<T>.FindLastIndex
IndexOf
overloads that matchList<T>.IndexOf
public void Insert (int index, T item)
to insert item at particular pointpublic void InsertRange (int index, IEnumerable<Point> points)
to insert a range of pointsLastIndexOf
overloads that matchList<T>.LastIndexOf
public void RemoveAt (int index)
to remove the Point at a given indexpublic void RemoveRange (int index, int count)
to remove a range of values (like the List function does)Reverse
overloads that matchList<T>.Reverse
Sort
overloads that matchList<T>.Sort
Modifications
public void Add(IEnumerable<Point> positions)
topublic void AddRange(IEnumerable<Point> positions)
public void Add(Rectangle rectangle)
topublic void AddRange(params Rectangle[] rectangles)
public void Add(IReadOnlyArea area)
topublic void AddRange(params IReadOnlyArea[] areas)
public void Remove(Func<Point, bool> predicate)
topublic void RemoveAll(Func<Point, bool> predicate)
public void Remove(HashSet<Point> positions)
topublic void RemoveAll(HashSet<Point> positions)
public void Remove(public void Remove(IEnumerable<Point> positions)
topublic void RemoveAll(IEnumerable<Point> positions)
public void Remove(IReadOnlyArea area)
topublic void RemoveAll(params IReadOnlyArea[] areas)
public void Remove(Rectangle rectangle)
topublic void RemoveAll(params Rectangle[] rectangles)
Removals
Positions
property (replaced by equivalent functions above andIEnumerable
implementation)PositionsSet
property (replaced by equivalent functions above andIEnumerable
implementation)Notable Absent Functions
List<T>.ConvertAll
-- Point does not support inheritance so there is little point to thisList<T>.CopyTo
-- Uncommonly used and typically you would not copy from Area to ArrayList<T>
.FindAll` -- System.LINQ can do this type of operation and not allocate as much memoryList<T>
.ForEach: can be done using typical foreach loop and system.linqList<T>.GetRange
-- can be done via LINQ construction and makes little sense to copy Areas like thisList<T>.Exists
-- this can be done via LINQList<T>.TrueForAll
-- Can be done vis LINQHashSet
functions: most of those operations don't seem to make sense to do to an area, or are covered by moreArea
-specific terminology likeIntersection
,Union
This is a very large restructure, so feedback is welcome; if it seems too extreme, feel free to say.