Skip to content
This repository has been archived by the owner on Feb 2, 2018. It is now read-only.

Add ListModel subclass #119

Merged
merged 3 commits into from Mar 27, 2017
Merged

Add ListModel subclass #119

merged 3 commits into from Mar 27, 2017

Conversation

mbarnes
Copy link
Contributor

@mbarnes mbarnes commented Mar 24, 2017

The SecretModel thing in #102 gave me an idea. I quite like how this turned out; tickles my object-oriented sensibilities.

ListModel is a subclass for model types represented as a JSON list.

Splitting the class hierarchy this way has some nice properties:

  • All classes that inherit directly from Model are represented as a JSON object/dictionary.
  • The _list_attr and _list_class class attributes move to ListModel, leaving Model less cluttered.
  • The Model.__str__() method can assume a dictionary structure.
  • The ListModel.__str__() method overrides this and assumes a list.
  • The Model._struct_for_json() method can assume a dictionary structure, eliminating the need for _dict_for_json().
  • The ListModel._struct_for_json() method overrides this and assumes a list, eliminating the need for _list_for_json().
  • The _json_type class attribute is no longer needed.
    Instead of this: if model._json_type == 'list':
    do this: if isinstance(model, ListModel):

Subclass for model types represented as a JSON list.

Splitting the class hierarchy this way has some nice properties:

 - All classes that inherit directly from Model are represented as
   a JSON object/dictionary.

 - The _list_attr and _list_class class attributes move to ListModel.

 - The Model.__str__() method can assume a dictionary structure.

 - The ListModel.__str__() method overrides this and assumes a list.

 - The Model._struct_for_json() method can assume a dictionary
   structure, eliminating the need for _dict_for_json().

 - The ListModel._struct_for_json() method overrides this and assumes
   a list, eliminating the need for _list_for_json().

 - The _json_type class attribute is no longer needed.

   Instead of this: if model._json_type == 'list':
           do this: if isinstance(model, ListModel):
The etcd handler doesn't need to worry about non-list models.
I don't think we need to introduce such a function yet.  The one place
where it was called, it wasn't really needed.
@mbarnes mbarnes requested a review from ashcrow March 27, 2017 02:50
Copy link
Collaborator

@ashcrow ashcrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Why didn't we think of this sooner 😄

@ashcrow ashcrow merged commit 88849c6 into projectatomic:master Mar 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants