Fattorino/ImNodeFlow

Null checking when find pin by UID fails

Closed this issue · 6 comments

Getting a Pin that doesn't exist simply crashes the program

Wouldn't this be the expected behaviour? Un-handled exception leads to program crash?

One improvement that could be made in this area is making the getInVal match up with addOUT/addIN via taking a std::string instead of a const char* or vice versa.

Yes, this is the expected behavior but I'm not happy with how I implemented Pin's value retrieval so I'll do a little experimentation to see if I find something better.

Also, getInVal is a template so I'm not sure what you mean by that.

commit 0cbbf51 closes this issue

Might have been mixing that up with something else I'll take another gander.

Yeah so I see it's partial template specialization so that it converts to std::string. I guess do we want those specializations to match?

addIN and addOUT hash a std::string to create a UID, but when getting a value from a template something like "foo" becomes a char* and therefore the hash doesn't match even if the string does. So I created that one specialization to go back to std::string.
Regarding the null checking, I decided to simply add an assertion to make it crash more nicely. Because I agree that a crash is the intended behavior. I'll also add more ways to check if a pin exists because they might be useful.

Wondering if just putting a std::string or std::string_view as both partial specializations would be sufficient. If we could get away with string view it'd possibly be more efficient. I'll play around with it and see what comes of it