CharaChorder / nexus

CharaChorder's logging and analysis desktop app, supporting Linux, Windows, and macOS.
GNU Affero General Public License v3.0
18 stars 6 forks source link

Version should be its own type #98

Closed GetPsyched closed 9 months ago

GetPsyched commented 9 months ago

Currently, versions are managed with neat little methods (encode_version and decode_version) which interchange the version format between human readable and machine comparable.

This issue suggests a version having its own type class which implements three methods:

  1. Version.encode(): This method mimics the existing encode_version.
  2. Version.decode(): This method mimics the existing decode_version.
  3. Version.__str__(): This method will mimic decode_version and allow for printing a version as if it were a primitive type.

Rationale

  1. Enforcing the Single Responsibility Principle. The SQLiteBackend class' responsibility is to manage the backend, not the version. If the version if extracted out, multiple backends can use it without re-implementing their own encoding formats.
  2. Better comparison between version types and strings/integers.
GetPsyched commented 9 months ago

Please assign this issue to me if this change makes sense. I posted it here simply to judge whether this will be merged at all so that I don't code something that won't even be accepted.

GetPsyched commented 9 months ago

Here's a sample of what such a class might look like:

class Version:
    def __init__(self, version: int | str):
        if isinstance(version, int):
            self.version = version
        else:
            self.version = self.as_int(version)

    @staticmethod
    def as_int(version: str) -> int:
        major, minor, patch = version.split(".")
        return int(major) << 16 | int(minor) << 8 | int(patch)

    @staticmethod
    def as_semver(version: int) -> str:
        return f"{version >> 16}.{version >> 8 & 0xFF}.{version & 0xFF}"

    def __str__(self):
        return self.as_semver(self.version)

    def __lt__(self, other: Self | str | int) -> bool:
        if isinstance(other, Version):
            return self.version < other.version
        if isinstance(other, str):
            return self.version < self.as_int(other)
        if isinstance(other, int):
            return self.version < other

As you can see, having methods like __lt__ means that a version object can easily be compared with other versions that are just a string or an integer without needing to explicitly convert them.

Raymo111 commented 9 months ago

Excellent, go for it.

GetPsyched commented 9 months ago

Where should the class be defined? Perhaps src/nexus/types/version.py? I'm unsure where this will fit in in the current project structure.

Raymo111 commented 9 months ago

I think src/nexus/version.py should be good, it's not like we're going to add any more project-wide types.