HW 18 done, but without doc and test #30
Conversation
| (def pokemons-url (str base-url "/pokemon")) | ||
| (def type-path (str base-url "/type")) | ||
| (def type-url (str base-url "/type")) | ||
| (def cache-wrapper |
There was a problem hiding this comment.
Больше похоже не на cache-wrapper, а на caching-request.
Чтобы улучшить читаемость кода (уменьшить вложенность) я бы предложил вытащить атом в def наружу (он в любом случае существует в единственном экземпляре) и, если есть желание, чтоб это был все-таки wrapper, то можно передавать функцию, которая делает запросы как аргумент сюда.
При текущем подходе все работать будет, так что эту часть в любом случае считаю выполненной.
There was a problem hiding this comment.
Дополнительно обращу внимание, что если нужно сохранять состояние атома при работе в репле, можно использовать defonce - это достаточно распространенная практика
| entities-parsed>)) | ||
|
|
||
| (defn construct-type-lang-hierarchy [name m] | ||
| [(get-in m [:language :name]) |
There was a problem hiding this comment.
(-> m :language :name) работает чуть быстрее get-in. Обычно get-in используется, когда ключи лежат в варах, или в качестве них используются строки/числа. Не ошибка, но лучше поменять.
| {name (get-in m [:name])}]) | ||
|
|
||
| (defn parse-type-names [response] | ||
| (let [name (:name response)] |
There was a problem hiding this comment.
опять же не ошибка, но я бы не рекомендовал использовать name в качестве байндинга, т.к. name это функция из кора и использование этого символа может привести к странным проблемам в дальнейшем
| (extract-types-by-lang lang))] | ||
| (->> (async/take limit (get-entities-data pokemons-url parse-pokemon-types)) | ||
| (async/into {}) | ||
| <!! |
There was a problem hiding this comment.
основная проблема в том, что в начале запросятся список покоменов, лишь затем дополнительные данные по ним, это можно делать асинхронно. Ок, если так и задумано, такое решение имеет место, но если цель была асинхронно запрашивать по мере поступления данных, то работать будет не совсем так.
|
|
||
| (defn extract-types-by-lang [lang types-map] | ||
| ;TODO transducer smells below | ||
| (partial mapv ((reduce (partial merge-with into) {} (vals types-map)) lang))) |
There was a problem hiding this comment.
я бы рекомендовал этот кусок кода упростить, понять, что тут происходит возможно, но читаются такой код достаточно тяжело, особенно учитывая, что он не делает экстракт, а возвращает экстрактор)
| [& {:keys [limit lang] :or {limit 50 lang "ja"}}]) | ||
| [& {:keys [limit lang] :or {limit 50 lang "ja"}}] | ||
| (let [pokemon-types (->> (get-entities-data type-url parse-type-names) | ||
| (async/into {}) |
There was a problem hiding this comment.
тут в любом случае будут запрашиваться и складываться в кеш все типы, я бы не рекомендовал так делать, например, на коротких выборках большинство из них может не понадобится в реальном примере.
Вместо этого можно сходить за типом по мере необходимости.
| (->> (async/take limit (get-entities-data pokemons-url parse-pokemon-types)) | ||
| (async/into {}) | ||
| <!! | ||
| (fmap pokemon-types) |
There was a problem hiding this comment.
интересно, почему решили использовать fmap, а не обычный map
Извините, что без комментов, постараюсь попозже добавить. На счёт значений размеров буфферов и параллелизма для пайплайнов не думал вовсе, кроме случая с вытаскиванием страниц, остальные от балды поставил. Кэширование сделал, но без персиста в бд, тоже может потом добавлю.