bschoening commented on code in PR #1273:
URL:
https://github.com/apache/cassandra-python-driver/pull/1273#discussion_r2895728241
##########
cassandra/util.py:
##########
@@ -1757,48 +1746,32 @@ def __repr__(self):
def __str__(self):
return self._version
- @staticmethod
- def _compare_version_part(version, other_version, cmp):
- if not (isinstance(version, int) and
- isinstance(other_version, int)):
- version = str(version)
- other_version = str(other_version)
-
- return cmp(version, other_version)
-
def __eq__(self, other):
if not isinstance(other, Version):
return NotImplemented
return (self.major == other.major and
self.minor == other.minor and
self.patch == other.patch and
- self._compare_version_part(self.build, other.build, lambda s,
o: s == o) and
- self._compare_version_part(self.prerelease, other.prerelease,
lambda s, o: s == o)
+ self.build == other.build and
+ self.prerelease == other.prerelease
)
def __gt__(self, other):
if not isinstance(other, Version):
return NotImplemented
- is_major_ge = self.major >= other.major
- is_minor_ge = self.minor >= other.minor
- is_patch_ge = self.patch >= other.patch
- is_build_gt = self._compare_version_part(self.build, other.build,
lambda s, o: s > o)
- is_build_ge = self._compare_version_part(self.build, other.build,
lambda s, o: s >= o)
-
- # By definition, a prerelease comes BEFORE the actual release, so if a
version
- # doesn't have a prerelease, it's automatically greater than anything
that does
- if self.prerelease and not other.prerelease:
- is_prerelease_gt = False
+ if self.major != other.major:
+ return self.major > other.major
+ elif self.minor != other.minor:
+ return self.minor > other.minor
+ elif self.patch != other.patch:
+ return self.patch > other.patch
+ elif self.build != other.build:
+ return self.build > other.build
+ elif self.prerelease and not other.prerelease:
+ return False
Review Comment:
@absurdfarce this is purely a stylistic choice, but I found the if-elif
chain a bit hard to read. It also helps from a data grouping perspective to
think of the version as a tuple and not separate variables.
I asked my Gemini about this and got the following:
> Using a sequence of if-else blocks (often called "cascading comparisons")
creates a "staircase" of logic that is hard to follow. Tuple comparison
expresses the intent in a single line.
>
> In Python, tuple comparison is almost always the stylistically superior
choice. It aligns with the "Pythonic" philosophy of writing code that is
readable, concise, and leverages the language's built-in power.
But that said, either way is fine, and I appreciate the way the PR cleans up
and simplifies the Version logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
