Refactoring is challenging but necessary. I feel like a broken record however, it is an important lesson that is being hammered into my mind. I would say it takes me about the same time to refactor as it does to create functioning code.
It is tough uncoupling the tightly wound code. But once the code is uncoupled it is amazing. Now I can add to the existing abstract code and I will not need to change too much.
Now we are here at my coupled code. I am currently using the EDN database, along with the JSON database, and I will soon be using the PostgreSQL database as well. I need to get into the habit of writing my code so that it has the option to simply add on databases or anything similar without adding too many unnecessary functions or coupled code.
I hid my fetch-the-games multimethod to focus on my different logs.
(def log-edn (atom (fetch-the-games :edn)))
(def log-json (atom (fetch-the-games :json)))
(defmulti save (fn [_game db-type] db-type))
(defmethod save :edn [game _db-type]
(->> game
(swap! log-edn assoc (:game-id game))
pr-str
(spit "log.edn")))
(defmethod save :json [game _db-type]
(->> game
(swap! log-json assoc (:game-id game))
json/write-str
(spit "log.json")))
As you can see there are a couple of no nos.
First is I am downloading 2 databases even though I will only use 1.
I will create a load function in order to just load in the necessary log.
Second, I am tightly coupled to both log-ed and log-json. I have instances of this coupling throughout my code.
(defmulti all-games (fn [db-type] db-type))
(defmethod all-games :json [_db-type] @log-json)
(defmethod all-games :edn [_db-type] @log-edn)
(defn get-game-by-id [game-id db-type]
(get (all-games db-type) game-id))
(defn last-game-id [db-type]
(max-game-id-edn (all-games db-type)))
(defn get-last-game [db-type]
(get (all-games db-type) (last-game-id db-type)))
I am passing through the datatype in order to select which log I am using. This is probably breaking multiple SOLID principles but in my mind this is breaking the Dependency Inversion Principle. Meaning I am relying on low level concrete detail and not abstraction.
I will focus on my first problem and that will help fix my second problem.
(def log (atom {}))
(defn load-db [db-type]
(reset! log (fetch-the-games db-type)))
Now I can just load in the database and set log to whichever database I am using. And I can set log as an atom and call it without having to make it an argument.
(defn get-game-by-id [game-id] (get @log game-id))
(defn last-game-id [] (max-game-id-edn @log))
(defn get-last-game [] (get @log (last-game-id)))
This looks simple now, but it can be tough to refactor.
Changing the tests, following the correct paths to find all of the places that have your coupled code.
The great thing is now that it is clean, I can just add in the PostgreSQL database into this no problem!
Best,
Merl