Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style guide: return empty string with String vs. null with String? #5737

Open
MaryaBelanger opened this issue Apr 19, 2024 · 2 comments
Open
Assignees
Labels
a.language Relates to the Dart language tour e2-days Can complete in < 5 days of normal, not dedicated, work p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. st.triage.ltw Indicates Lead Tech Writer has triaged

Comments

@MaryaBelanger
Copy link
Contributor

(From a thread in Dart Discuss chat) Do we need a style guide entry (or maybe a lint?) about "having a return type of String, and returning empty string, vs having a return type of String?, and returning null?" (Or is there one already? I didn't find anything)

IMO it's almost always better to return String?. It's safer to return "something that might not be a string" (String?) vs. "something that always looks like a string" (''). Dart will make you always explicitly handle the former, but it's easy for the latter to slip through the cracks.

I think it is better to return String? as activation date. If there is no activation, the correct result is null.
It might be useful to have a lint that does not allow you blindly interpolate nullable values, so you don't get "Activated at: null".

@MaryaBelanger MaryaBelanger self-assigned this Apr 19, 2024
@atsansone atsansone added a.language Relates to the Dart language tour p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. e2-days Can complete in < 5 days of normal, not dedicated, work st.triage.ltw Indicates Lead Tech Writer has triaged labels Apr 23, 2024
@natebosch
Copy link
Member

I don't know if we've identified any consensus view for this since authors have been working with null safe Dart. cc @lrhn @munificent

If we want to be prescriptive I don't think I have a strong opinion towards either pattern.

@lrhn
Copy link
Member

lrhn commented Apr 23, 2024

I'm strongly on the "use null to represent no value" camp.

If all string values are valid values, then you have no choice.

If not all string values are valid, then you can use an invalid string as a sentinel to represent "no valid value". Or you can use null. I see absolutely no advantage in using a string. It's a mistake to think that it's more efficient, because you don't have to check for null before using the value, if you have to check for the sentinel first anyway. And relying on "promotion" by checking a field for not being the sentinel, and then being able to use it as a String afterwards without using !, is something we have plenty of ways to do with String? today. And actually safely.

If the empty string is one of the invalid values, you can use that.
But it's still arbitrary. You wouldn't use "__*__" as the sentinel, just because no valid string can start and end with "__". Why not? Because it's arbitrary and unnecessary, just use null. It works every time. That's what it's there for.

The only thing that the empty string has going for it, compared to another sentinel string, is that s.isEmpty is shorter than s == _noValue. Not more readable, though.
I'd count against the empty string that it doesn't show up if you accidentally include it in an output.
And if it's not the only invalid string, you may still have to validate the rest, and now the empty string is "valid" as an exception.

Use null to represent "no value". Do it consistently, because it's the one thing that always works. Don't try to "optimize" and use non-null values in some cases, and not in others. It's inconsistent, and not actually more efficient.

Special case: if "no value" is itself a domain value, like an enum with a noValue element, then there is no need for a "no value" representation outside of the type. Strings are not such a case. The string representation of such an enum is also not such a case, just treat its values consistently.

Special case: integers. Especially ones always on the smi range, might actually be more efficient using -1 as sentinel than using null.
(But only worry about this in high performance code. Dart String operations like indexOf should have returned null.)

So I completely agree with the quotes. Using null is safer (actually prevents using as a real value), easier to work with (can use any of the null-aware operators), and more consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a.language Relates to the Dart language tour e2-days Can complete in < 5 days of normal, not dedicated, work p3-low Valid but not urgent concern. Resolve when possible. Encourage upvote to surface. st.triage.ltw Indicates Lead Tech Writer has triaged
Projects
None yet
Development

No branches or pull requests

4 participants