TommyKaneko / Sketchup-API-C-Wrapper

A complete set of C++ Wrapper classes for SketchUp C API objects
MIT License
29 stars 8 forks source link

!operators #3

Open thomthom opened 7 years ago

thomthom commented 7 years ago

https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/blob/master/SUAPI-CppWrapper/model/Edge.hpp#L114

I'm not sure if having the ! operator implemented. I think code would read better if you have to call edge.IsValid(). Having that operator there almost makes it seem like a pointer.

TommyKaneko commented 7 years ago

I never thought about the confusion with pointers. I suppose I am coming from a Ruby background, where ! would make sense as NOT.

Having said this, I feel that .isValid() feels more C than C++ (with my limited experience, that is). I am inclined to be more inviting to Ruby programmers than C programmers.

Given that the method, whether .isValid() or operator!, actually checks if the underlying pointer in the object is nullptr or not, perhaps there is little room for confusion. I think the best solution is to provide both method and overload.

I personally will miss the very shorthand double exclamation mark e.g. "!!edge" to check validity of an object.

thomthom commented 7 years ago

Oh, it's checking if the Ref itself is valid... hm...

I'm still not convinced this is a good idea, because it's easy to get confused about whether we're talking about a valid Ref or an erased entity. Need to think about this one.

TommyKaneko commented 7 years ago

It uses the SUIsValid() SUIsInvalid() C functions.

At the moment, you can't erase entities through the C API, so there's no confusion there. But I guess that could change in future.

Have a look st the implementation on Entity.cpp

Sent from my iPhone

On 27 Oct 2017, at 09:36, Thomas Thomassen notifications@github.com wrote:

Oh, it's checking if the Ref itself is valid... hm...

I'm still not convinced this is a good idea, because it's easy to get confused about whether we're talking about a valid Ref or an erased entity. Need to think about this one.

― You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.