RhinoCommon impure methods

Hi @stevebaer

I think we should start adding the [Pure] attribute to RhinoCommon methods which do not change underlying states. I’m becoming a big fan of readonly fields and immutable classes, and ReSharper isn’t happy about calling methods like DistanceTo() on readonly Point2d fields.

Is this something we should add piece-meal or would it make more sense to try and do all the work at once?

I wouldn’t really recommend ever exposing fields. Do you mean read only properties?

I actually didn’t know about this attribute. I like the idea, but don’t know enough about [Pure] to understand how it is used. Is it informational only or does the compiler make sure that you can’t go tweaking a field in something marked as [Pure]? Guess I could run some tests myself, but I’m just being lazy :smirk: In C++, we use the const keyword for this purpose and the compiler actually helps us enforce the fact that we should not be allowed to change things.

Along the same vein, we should also consider adding information that informs the developer that something is (or is not) threadsafe. I see that there is a comment we can add for documentation

Doing all of this at once is probably not feasible. Maybe do a whole source file at once so we know that a file has been evaluated instead of willy-nilly additions.

I never expose fields, however the class itself will need access to private readonly fields and it should be able to call pure methods on those fields. From what I understand a pure method in .NET is basically the same as a const method in C++, though with the added constraint that it is purely dependent on the inputs, not any externalities.

Point2d.DistanceTo() qualifies as it does not change the X, Y or Z values of the point.

Doing it on a per-file or per-class basis works for me.

I don’t feel qualified to make any decisions on thread-safety. not only do I not know enough about the C++ part of the Rhino SDK, I’m not even sure I know about .NET multi-threading to adjudicate.

For testing, I made a function in a class that changed field values and marked that function as [Pure]. The compiler didn’t complain at all which makes me wonder when this attribute’s purpose is. Is it just informative or am I confused?

If you start adding this, I would also like to request for using the JetBrains Annotation Framework with attributes [NotNull], [CanBeNull], [StringFormatMethod] and [ContractAnnotation] (and possibly several others). These attributes are understood by the ReSharper Visual Studio Add-In, which reasons about code (code analysis) using heuristics based on these attributes.

http://www.jetbrains.com/resharper/webhelp/Code_Analysis__Code_Annotations.html

I think you’re right. It’s used in R# but it doesn’t seem to solve the issue I was having. So let’s not pepper RhinoCommon with [Pure]…