search

MagicTheGathering / mtg-sdk-javascript

Magic: The Gathering SDK - Javascript
MIT License
286 stars 62 forks source link

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

Open dkniffin opened 7 years ago

dkniffin commented 7 years ago

I've been having a discussion over in https://github.com/mtgjson/mtgjson/issues/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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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