emiln / majic Goto Github PK

The parsed cards should be stored in a datomic database.

I expect this is due to agents surviving their usefulness. Perhaps a simple call to shutdown-agents can fix it.

5 of the 15 tests currently fail. This will clearly need to be fixed. Output from Codeship.io:

FAIL "About string->int - It should parse a bunch of numbers." at (util_test.clj:18)
    Expected: 9013593195
      Actual: nil

FAIL "About n-partitions - It should partition unevenly if necessary." at (util_test.clj:26)
    Expected: [[0 3 6 9] [1 4 7] [2 5 8]]
      Actual: ((0 3 6 9) (1 4 7 nil) (2 5 8 nil))

FAIL "About documentation. - No public mappings in the parser can be missing docstring." at (documentation_test.clj:38)
    Expected: #{}
      Actual: #{->string all-card-ids card-by-id exp-string exp-string-left exp-string-right id-string id-string-left id-string-right url}

FAIL "About documentation. - No public mappings in the util namespace can be missing docstring." at (documentation_test.clj:40)
    Expected: #{}
      Actual: #{file->int-seq string->int string->keyword}

FAIL "About the -main function - It should print a sensible error message when called without arguments." at (core_test.clj:9)
    Expected: #".+"
      Actual: java.lang.NumberFormatException: null
              majic.core$_main.doInvoke(core.clj:7)
              majic.core_test$fn__10287$base_function__5041__auto____10291$fn__10292$base_function__5041__auto____10293$fn__10294$fn__10295.invoke(core_test.clj:9)
              majic.core_test$fn__10287$base_function__5041__auto____10291$fn__10292$base_function__5041__auto____10293$fn__10294.invoke(core_test.clj:7)
              majic.core_test$fn__10287$base_function__5041__auto____10291$fn__10292$base_function__5041__auto____10293.invoke(core_test.clj:7)
              majic.core_test$fn__10287$base_function__5041__auto____10291$fn__10292.invoke(core_test.clj:7)
              majic.core_test$fn__10287$base_function__5041__auto____10291.invoke(core_test.clj:7)
              majic.core_test$fn__10287.invoke(core_test.clj:7)

5 failures, 0 errors.
>>> Midje summary:
FAILURE: 5 checks failed.  (But 10 succeeded.)
Subprocess failed

We'll need web server library to serve cards to application programmers. I am not intimately familiar with the Clojure web stacks, so this will be an open question for now.

There are currently no tests for the project. I propose using Midje and behavior-driven development (or at least testing.)

As far as what to actually test, I suggest the following for a start:

• "Regular" tests:
  • Do all public bindings in our namespaces have docstrings?
  • Do the various functions in majic.util behave as expected?
• "Full" tests:
  • Does the return value of card-by-id contain non-nil data for the expected keys for a handful of chosen card IDs?
  • Does all-card-ids return a sequence of integers?

As a developer of the majic project, I really need to be able to retrieve data from the Gatherer database. The specifics of the format returned by the parser is an open question. I present a suggestion below.

Evaluating (query-cards some-query) could return data in a format similar to the following:

[{:artist "Nils Hamm"
  :flavor
  "For the necromancer barons, killing and recruitment are one
   and the same."
  :expansions {:placechase :rare}
  :mana-cost
  {:black 2 :converted 3 :neutral 1}
  :name "Death Baron"
  :power 2
  :rules
  ["Skeleton creatures you control and other Zombie creatures
    you control get +1/+1 and have deathtouch."]
  :toughness 2
  :types [:creature :zombie]}
 {:artist "Randy Gallegos"
  :flavor "For the Scuzzback gang, it's always \"raid o'clock.\""
  :expansions {:shadowmoor :rare}
  :mana-cost
  {:converted 4 :neutral 3 #{:green :red} 1}
  :name "Impromptu Raid"
  :rules
  [{:neutral 2 #{:green :red} 1}
   "Reveal the top card of your library. If it isn't a creature
    card, put it into your graveyard. Otherwise, put that card
    onto the battlefield. That creature gains haste. Sacrifice it
    at the beginning of the next end step."]}]

All data is available from Gatherer, but the rules parsing may be particularly difficult.

The parser currently (and in my opinion mistakenly) always serializes the parsed set of magic cards to a file when parsing all cards. I think it would be more pleasing to have majic.parser/all-cards work in the following fashion:

• Signature becomes [& opts], where options include:
  • :serializer-fn, which defaults to an EDN writer writing to disk.
  • :logging-fn, which defaults to println.
  • :parallel-downloads-limit, which defaults to 50.
• Instead of using pmap, use map with future to increase the number of concurrent threads.

The parser currently uses pmap and slurp to handle its parallel HTTP requests when fetching all cards. This is unlikely to be the fastest means of performing many simultaneous requests.

I think http-kit client could vastly improve the performance of majic.parser/all-cards if used right. As far as my understanding goes, pmap uses a thread pool of size n + 2, where n is the number of cores in your CPU. Clearly this is not a CPU-bound scenario, and I think this number should be dramatically increased.

The namespace is currently a great example of how to repeat yourself constantly throughout the code. This is clearly bad and in blatant violation of the DRY principle.