-
Notifications
You must be signed in to change notification settings - Fork 8
finish 8th homework #42
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
хорошая работа
мне поправилось ваше решение
предлагаю в качестве разминки добавить еще немного тестов для основных функций
otus-16/src/otus_16/homework.clj
Outdated
(->> log-file | ||
(line-seq) | ||
(partition-all 5000) | ||
(pmap #(map parse-log %)) | ||
(filter-by-url url) | ||
(filter-by-referer referrer) | ||
(sum-partition))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
хорошее решение, мне понравилось
пришлось потратить немного времени чтобы понять все трансформации
хорошей практикой считается не сильно изменять тип передаваемого аргумента в цепочке вызовов тред макроса, т.е. если вы начали работать с коллекцией то в том месте где коллекция трансформируется во что-то другое лучше закончить цепочку вызовов (в данном случае когда коллекция из плоской превращается в двухмерную, после вызова pmap)
мне кажется если после pmap сделать вызов flatten и дальше работать как с простой коллекцией код можно сделать немного легче для чтения
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
поправил как смог )
flatten
делает Array.copyOf
, что приводит к
Execution error (OutOfMemoryError) at java.util.Arrays/copyOf (Arrays.java:3537).
Java heap space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
спасибо за работу
всё лаконично и понятно
оставил комментарий по поводу перехвата ошибок из :pre блока, в остальном всё отлично 👍
(defn get-logs-paths | ||
[dir-path] | ||
(->> (io/as-file dir-path) | ||
(file-seq) | ||
(filter #(.isFile %)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
(defn ->url [log] | ||
{:pre [(contains? log :request)]} | ||
(let [request (-> log :request)] | ||
(when request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а для чего тут when используется? ведь у функции есть проверка в блоке :pre
(map #(io/reader %) logs)) | ||
|
||
(defn ->url [log] | ||
{:pre [(contains? log :request)]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
с pre и post условиями есть один нюанс - если (contains? log :request)
вернет false будет выброшенно исключение так что везде где эти функции вызываются надо добавлять try/catch
No description provided.