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

[MaryaBelanger]

(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".

[natebosch]

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]

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.