search

Fattorino / ImNodeFlow

Node based editor/blueprints for ImGui
MIT License
119 stars 15 forks source link

Null checking when find pin by UID fails #7

Closed Fattorino closed 4 months ago

Fattorino commented 4 months ago

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

avlec commented 4 months ago

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.

Fattorino commented 4 months ago

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.

Fattorino commented 4 months ago

commit 0cbbf51 closes this issue

avlec commented 4 months ago

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?

Fattorino commented 4 months ago

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.

avlec commented 4 months ago

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