Alright, time for a little recap. Several pull requests have been merged to fix some of the above issues. Check the original post for an updated list. However there are a few pull requests that need some additional discussion.
With the pull request that changes
GetPointCount() and
SetPointCount() to a property a few things came up. The shape base class only defines the property with a getter and no setter. So in the circle and convex shapes we also need the setter. But in C# it is impossible to override a property and also add a setter if the base class does not define a setter. So there is 4 options to solve this.
#1 Instead of overriding the PointCount property in derived classes, simply declare the property as 'new'. This will allow the derived classes to implement getters or setters as needed, however it has the drawback of when a reference just to the base class type, any calls will not call the derived type's property. And then this breaks the inheritance model, so it is a nono from me.
#2 Go with how I setup the pull request, change
GetPointCount to a property and then leave the function
SetPointCount in the derived classes that allow changing the point count. The drawback here is that it is inconsistent. To get the point count the property is used, but to set a point count the function is used.
#3 Another option is built off of #1 and #2. Change both functions to a property and in the base shape class and just have the setter part of the function not do anything. This would allow derived classes to override the setters and/or getters without any issue and everything will work properly when dealing with base class types. The only drawback to this solution is that setting the point count may not have any affect and this may cause some confusion.
#4 Or leave as is with the functions for consistency.
With this pull request I changed the
SetView() and
GetView() functions to a property. This issue with this is that we are wondering if it hides the true intent of of the View property. What I mean is that is it clear enough that you must set the view after changing it.
Take for example the following code.
// Current working codevar view
= window
.GetView();view
.Move(new Vector2f
(5,
5));window
.SetView(view
);// New property that would work// the question is whether or not this View property would be clear enough that you must assign it after changing itvar view
= window
.View;view
.Move(new Vector2f
(5,
5));window
.View = view
;// However I think both methods fall to this problemwindow
.GetView().Move(new Vector2f
(5,
5));window
.View.Move(new Vector2f
(5,
5)); Please give feedback on these issues.