Null or empty string when GetUserString fails?

[goswinr]

The docs say it returns null on failure, but it seems to return an empty string. I think actually returning NULL would be better. the key might exist with an empty string as value. no ?

https://github.com/mcneel/rhinocommon/blob/57c3967e33d18205efbe6a14db488319c276cbee/dotnet/opennurbs/opennurbs_object.cs#L436

[piac]

Hi @goswinr that is an internal method. However, as you say, it's used in the public SDK. It ends calling into StringHolder.GetString: https://github.com/mcneel/rhinocommon/blame/57c3967e33d18205efbe6a14db488319c276cbee/dotnet/opennurbs/opennurbs_array.cs#L121 Because that is something @sbaer added, he might want to comment if that was on purpose or a minor slip. I can fix it in the second case.

[sbaer]

I'm fine with either solution (change the code or the comment). Changing the comment does minimize breaking anything that may be existing though.

[hstehling]

Hi, I stumbled upon this discussion and just want to add my 2 cents: We have lots of Python code checking if certain user strings exist. While I encourage people to use generic checks like if not str instead of if str == "", I am pretty sure that changing the return value will break stuff. I agree with @goswinr that null would be the nicer return value, but the change should not be done "by the way"... (so, maybe for Rhino 6?)

[goswinr]

@sbaer I agree changing the docs is safer. What is the way to check if a key does not exist or exists and has an empty string as value? could there be a new method for that? like bool IsKey(string)

[goswinr]

@piac it would affect the result of the python method rs.GetUserText(key) too.

[piac]

I've given this a few thoughts and I agree it's a disputable decision both ways. Normally I would try to maintain as much a possible current behavior... I usually "fight" in that direction. However, this time it seems different. To me, the non-null return is just, after all, a bug. It's documented otherwise, and it's supposed to be returning "nothing", not a string that happens to be void of chars.

So, this will change for Rhino WIP/Rhino 6 to returning None/null/Nothing -- and remain the same in Rhino 5. This will give time for possible bugs to surface, show if this was a wise decision, and provide a little extra safety for current V5 scripts.

Thanks for reporting and discussing this!

[hstehling]

Thanks, @piac. @goswinr: Rhino does not allow you to set a user string with an empty string as either key or value. SetUserString returns False in this case. So imho it is safe to assume an empty string from GetUserString means "Key does not exist".

[piac]

https://mcneel.myjetbrains.com/youtrack/issue/RH-39612