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

HW 18 done, but without doc and test #30

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion otus-18/project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
[org.clojure/core.async "1.6.673"]
[clj-http "3.12.3"]
[clj-http-fake "1.0.4"]
[cheshire "5.11.0"]])
[cheshire "5.11.0"]
[org.clojure/algo.generic "0.1.3"]])
154 changes: 142 additions & 12 deletions otus-18/src/otus_18/homework/pokemons.clj
Original file line number Diff line number Diff line change
@@ -1,19 +1,149 @@
(ns otus-18.homework.pokemons)
(ns otus-18.homework.pokemons
(:require
[clojure.core.async
:as async
:refer [chan <!! >!! <! >! go pipeline-async close! thread pipeline]]
[clj-http.client :as http]
[clojure.algo.generic.functor :refer [fmap]]))

(def base-url "https://pokeapi.co/api/v2")
(def base-url "https://pokeapi.co/api/v2")
(def pokemons-url (str base-url "/pokemon"))
(def type-path (str base-url "/type"))
(def type-url (str base-url "/type"))
(def cache-wrapper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Больше похоже не на cache-wrapper, а на caching-request.
Чтобы улучшить читаемость кода (уменьшить вложенность) я бы предложил вытащить атом в def наружу (он в любом случае существует в единственном экземпляре) и, если есть желание, чтоб это был все-таки wrapper, то можно передавать функцию, которая делает запросы как аргумент сюда.
При текущем подходе все работать будет, так что эту часть в любом случае считаю выполненной.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дополнительно обращу внимание, что если нужно сохранять состояние атома при работе в репле, можно использовать defonce - это достаточно распространенная практика

(let [cache (atom {})]
(fn [http-map]
(if-let [cached-result (get @cache http-map)]
cached-result
(let [result (http/request http-map)]
(swap! cache assoc http-map result)
result)))))

(defn extract-pokemon-name [pokemon]
(:name pokemon))
(defn async-request [http-map]
(-> http-map
cache-wrapper
:body
thread))

(defn extract-type-name [pokemon-type lang]
(->> (:names pokemon-type)
(filter (fn [type-name] (= lang (-> type-name :language :name))))
(first)
:name))
(defn pageable->urls [pages-urls> url entities-urls>]
(go
(let [response (<! (async-request {:url url
:method :get
:as :json}))
next-url (:next response)]
(doseq [entity (:results response)]
(>! entities-urls> (:url entity)))
(if next-url
(>! pages-urls> next-url)
(close! pages-urls>))
(close! entities-urls>))))

(defn urls->entities [url entities-content>]
(go
(let [response (<! (async-request {:url url
:method :get
:as :json}))]
(>! entities-content> response)
(close! entities-content>))))

(defn get-entities-data [initial-page-url parse-fn]
(let [pages-urls> (chan 2)
entities-urls> (chan 8)
entities-data> (chan 8)
entities-parsed> (chan 8)]
(pipeline-async 1 entities-urls> (partial pageable->urls pages-urls>) pages-urls>)
(pipeline-async 8 entities-data> urls->entities entities-urls>)
(pipeline 8 entities-parsed> (map parse-fn) entities-data>)
(>!! pages-urls> initial-page-url)
entities-parsed>))

(defn construct-type-lang-hierarchy [name m]
[(get-in m [:language :name])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(-> m :language :name) работает чуть быстрее get-in. Обычно get-in используется, когда ключи лежат в варах, или в качестве них используются строки/числа. Не ошибка, но лучше поменять.

{name (get-in m [:name])}])

(defn parse-type-names [response]
(let [name (:name response)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

опять же не ошибка, но я бы не рекомендовал использовать name в качестве байндинга, т.к. name это функция из кора и использование этого символа может привести к странным проблемам в дальнейшем

[name
;TODO transducer smells below
(into {} (map (partial construct-type-lang-hierarchy name) (:names response)))]))

(defn parse-pokemon-types [response]
[(:name response)
(mapv #(get-in % [:type :name]) (:types response))])

(defn extract-types-by-lang [lang types-map]
;TODO transducer smells below
(partial mapv ((reduce (partial merge-with into) {} (vals types-map)) lang)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я бы рекомендовал этот кусок кода упростить, понять, что тут происходит возможно, но читаются такой код достаточно тяжело, особенно учитывая, что он не делает экстракт, а возвращает экстрактор)


(defn get-pokemons
"Асинхронно запрашивает список покемонов и название типов в заданном языке. Возвращает map, где ключами являются
"Асинхронно запрашивает список покемонов и название типов в заданном языке. Возвращает map, где ключами являются
имена покемонов (на английском английский), а значения - коллекция названий типов на заданном языке."
[& {: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 {})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут в любом случае будут запрашиваться и складываться в кеш все типы, я бы не рекомендовал так делать, например, на коротких выборках большинство из них может не понадобится в реальном примере.
Вместо этого можно сходить за типом по мере необходимости.

<!!
(extract-types-by-lang lang))]
(->> (async/take limit (get-entities-data pokemons-url parse-pokemon-types))
(async/into {})
<!!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

основная проблема в том, что в начале запросятся список покоменов, лишь затем дополнительные данные по ним, это можно делать асинхронно. Ок, если так и задумано, такое решение имеет место, но если цель была асинхронно запрашивать по мере поступления данных, то работать будет не совсем так.

(fmap pokemon-types)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

интересно, почему решили использовать fmap, а не обычный map

)))

(comment
(get-pokemons))

(comment

(fmap (partial mapv ((reduce (partial merge-with into) {} (vals (get-types))) "ja")) (get-pokemons))

(map #(get-in (get-types) [% "en"]) ["bug" "flying"])

(reduce (partial merge-with into) {} (vals (get-types)))


(defn- get-types
[& {:keys [limit lang] :or {limit 50 lang "ja"}}]
(let [pokemon-types (->> (get-entities-data type-url parse-type-names)
(into {})
<!!)]
pokemon-types))

(def pika
{:url (str pokemons-url "/" "pikachu")
:method :get
:as :json
:query-params {:lang "jp"}})

(def pika
{:url type-url
:method :get
:as :json})

(go (let [start# (. System (nanoTime))
pokemons-result (<! (async-request {:url pokemons-url
:method :get
:as :json
:query-params {:lang "jp" :limit 50}}))
pokemon-names (->> pokemons-result
:body
:results
(map :name))]
(println pokemon-names)
(println (str "Elapsed time: " (/ (double (- (. System (nanoTime)) start#)) 1000000.0) " msecs"))))


(time (-> (cache-wrapper pika)
:body
keys))

(let [ch> (async-request pika)]
(->> ch>
<!!))

(let [out> (get-entities-data type-url parse-type-names)]
(go-loop []
(if-let [res (<! out>)]
(do (println res) (recur))
"end")))

(get-pokemons)
)