Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper function for getting a card's static abilities #7

Open
dkniffin opened this issue Feb 13, 2017 · 3 comments
Open

Helper function for getting a card's static abilities #7

dkniffin opened this issue Feb 13, 2017 · 3 comments

Comments

@dkniffin
Copy link

I've been having a discussion over in mtgjson/mtgjson#285 about the best way to get a list of static abilities for a given card. Some examples: Rotting Rats would return ['Unearth'], Anax and Cymede would return ['First Strike', 'Vigilance']

I just finished up a helper function for doing this, and posted it in the issue I mentioned above. I think that would ideally go in this library. If you're interested, I can make a PR for it.

@raineorshine
Copy link
Collaborator

Nice!

I can imagine this being useful in two places:

  1. A query field in the API - This requires a change to magicthegathering.io.
  2. A helper function for the card objects that come back from the API - Naturally implemented in this library.

Regarding the implementation of (2), I can imagine this functionality being provided through an instance method on the card object (card.abilities()) or a static function on the root mtg object (mtg.abilities(card)). An instance method will be most convenient and familiar for the average object-oriented developer. The downside is that it changes the nature of the card object from static data (JSON object) to a hybrid object with both data and a function. This could be problematic for the serialization of card objects, for example.

I would like to hear your input on the above assessment, then I welcome your contribution!

@dkniffin
Copy link
Author

For my app, I'm downloading all the data before runtime, and just loading in a giant json blob, rather than hitting the API every time. So, in my case, I think it'd be more helpful to have this be a static method.

@raineorshine
Copy link
Collaborator

Okay, doesn't hurt to add it as a static method :)

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

No branches or pull requests

2 participants