Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

Class initialisation from JSON #82

Closed
richcooper95 opened this issue Jan 20, 2022 · 0 comments · Fixed by #79
Closed

Class initialisation from JSON #82

richcooper95 opened this issue Jan 20, 2022 · 0 comments · Fixed by #79

Comments

@richcooper95
Copy link

richcooper95 commented Jan 20, 2022

Is your feature request related to a problem? Please describe.
Currently, classes in duffel_api/models take a json param in the __init__ method and loop through everything in the dict to set the attrs.

This isn’t optimal for type-checking, and also makes the user experience worse because it’s unclear what the attributes of a class are for users of the library (they’d have to go and check the API docs in this case, but it’d be nice if they could get access to this info in the SDK - especially with on-hover definitions in many IDEs).

Describe the solution you'd like
A way around this is to reserve __init__ to explicitly list out all attributes of the class, and take all attributes as explicit keyword args. This can be used in conjunction with a .from_json(json) classmethod which unpacks the dict and passes it to the class initialiser.

  • At a glance, users can see the attributes (and types) for this class.
    • This is especially useful when your IDE supports docs in on-hover defs!
    • Add type hints for models #81 could also be addressed well at the same time.
  • All expected fields will raise a KeyError if not present in the JSON (since this would indicate our API wasn't returning some expected data).
  • All non-required fields will use a default.

As a toy example, this:

class OfferRequest:
    def __init__(self, json):
        for key in json:
            value = json[key]

            if isinstance(value, str):
                value = maybe_parse_date_entries(key, json[key])
                setattr(self, key, value)
                continue

            if isinstance(value, list):
                if key == "offers":
                    value = [Offer(v) for v in value]
                elif key == "passengers":
                    value = [Passenger(v) for v in value]
                elif key == "slices":
                    value = [Slice(v) for v in value]

            setattr(self, key, value)

could be changed to this:

class OfferRequest:
    def __init__(
        self,
        passengers: List[Passenger],
        slices: List[Slices],
        cabin_class: str,
        offers: List[Offer],
        live_mode: bool,
        offer_request_id: str,
        created_at: datetime.datetime,
):
    self.passengers: List[Passenger] = passengers
    self.slices: List[Slice] = slices
    self.cabin_class: str = cabin_class
    self.live_mode: bool = live_mode
    self.offer_request_id: str = offer_request_id
    self.created_at: datetime.datetime = created_at
    self.offers: List[Offer] = offers

    @classmethod
    def from_json(cls, json: dict):
        """Construct a class instance from a JSON response."""
        return cls(
            passengers=[Passenger(p) for p in json["passengers"]],
            slices=[Slice(s) for s in json["slices"]],
            live_mode=json["live_mode"],
            offer_request_id=json["offer_request_id"],
            cabin_class=json["cabin_class"],
            created_at=maybe_parse_date_entries("created_at", json["created_at"]),
            offers=[Offer[o] for o in json.get("offers", [])],
        )

Describe alternatives you've considered
The most obvious alternative is the way things are currently done (loop through a JSON dict, setting attributes as you go) - but it's way less clear to users of the SDK what attributes a given class has.

Additional context
n/a

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant