robotdotnet / WPILib

DotNet implementation of WPILib for FIRST Robotics Competition (FRC)
27 stars 8 forks source link

Change interfaces/classes to use .NET language features #15

Closed jkoritzinsky closed 9 years ago

jkoritzinsky commented 9 years ago

I've been working on porting over the CANTalon class, and I noticed that there are many methods that can seem like they should be converted into .NET properties. I've done the ones that I feel should be done in the CANTalon class in my working copy, but there are some in some common interfaces that really should be changed to properties. I think we should change them. What do you think?

ThadHouse commented 9 years ago

That ones interesting. I know the RobotPy guys decided to make all the externally facing functions be the same as what they are in Java and C++, so that if teams wanted to port their code they could do so easily. However, I do think that implementing Properties would make things much cleaner for DotNet.

So my thought would be to implement properties, but also potentially keep the old get and set functions, and mark them obsolete. That way if someone wants to port their code it will still work, but otherwise they can use properties. At least to start.

jkoritzinsky commented 9 years ago

I agree with that plan. Lets go for it.

jkoritzinsky commented 9 years ago

Also, I suggest that we implement IDisposable on the classes and mark the delete methods Obsolete.

ThadHouse commented 9 years ago

That sounds like a good idea. For starters, most sensors extend from SensorBase. So if you make SensorBase implement IDisposable and then make the Dispose function abstract in SensorBase. That way all classes that inherit from SensorBase have to implement a Dispose method.

jkoritzinsky commented 9 years ago

I'm just having Dispose call Free until we have a chance to switch it to the other order and fully implement the Dispose pattern.

ThadHouse commented 9 years ago

So looking through Java documentation, it looks like the free functions that are defined in most of the classes are not the ones that are automatically called by the GC. Only the finalize() methods. Looking around, it looks like only CANTalon implements finalize. So Java's garbage collector doesn't actually call the free() functions. Also looking around, It looks like implementing an actual finalizer in dotnet causes the GC to have to work harder, so it causes some slight performance issues. Do we just want to implement IDisposable, and not do the full finalizer pattern, or do we want to implement the full finalizer pattern, and hope it doesn't drop performance too much. I know most teams never actually free their ports anyway, and the ones that actually would probably know to call Dispose() anyway.

ThadHouse commented 9 years ago

Or the safer way might be to make a SafeHandle for all of the IntPtrs. Theres only 8-10 different structs, so it wouldn't take to much time to implement.

jkoritzinsky commented 9 years ago

We can just do the Dispose method instead of the full dispose pattern. Or SafeHandles. Whichever you prefer.

ThadHouse commented 9 years ago

Fixed in the C# 6.0 branch.