-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
Directly related to #121468.
Filing this as a sub-issue to be fixed with its own PR (this also affects non Android Studio-related code).
Context
Version is a class that is used to represent a version of multiple pieces of software used throughout the tool, including Android Studio, IntelliJ, Visual Studio Code, and Cocoapods.
Issue
Version has a null-object1 instantiation, Version.unknown, which is used in a few places to represent the flutter tool being unable to detect the version of some software. Because Version also implements Comparable, it is easy to write logic that compares a more typical version object (e.g. Version(1, 2, 3)) to Version.unknown, which doesn't semantically make sense. For a specific example, consider Version(0,0,0) == Version.unknown, which resolves to true.
Because of this, branching logic based on Version comparison is prone to error if Version.unknown is used. If someone uses Version.unknown and they compare Verison objects, they must remember to explicitly check for Version.unknown in addition to those comparisons. Otherwise, they are at risk of treating Version.unknown as a legitimate version (0.0.0). Again, see #121468.
Possible fixes
- Remove
Version.unknownentirely. The drawback is that tool code has to use something else to represent the inability to find/detect a version within a software installation.nullis the easiest choice, but the meaning ofnullis not as clear asVersion.unknown, and it results in slightly less readable code. - Make the
compareTofunction ofVersionaware ofVersion.unknown(which should be made into astatic const). Specifically, comparison withVersion.undefinedshould result in an exception being thrown. While arguably better than the current behavior, users still need to remember to check forVersion.unknownbefore making comparisons.
I suggest option 1.
Footnotes
-
I'm abusing terminology here, since this object doesn't have any null/default behavior, but it still accomplishes something a null-object is typically used to do—avoiding references to
null. ↩