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

Vadzimz homework 6 #22

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

Vadzimz
Copy link

@Vadzimz Vadzimz commented Jan 10, 2024

No description provided.

[keys-to-select key-to-first coll]
(->> coll
(map #(select-keys % keys-to-select))
(map #((fn [x key] (merge {(key x) (dissoc x key)})) % key-to-first))))
Copy link
Contributor

Choose a reason for hiding this comment

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

Как-то сложновато выглядит. Зачем тут вложенные лямбды? Кажется, что стоит одну (fn [x] ..) оставить.

(map (fn [[name vals]]
{name-key name
aggr-key (reduce + (map aggr-key vals))}))
(map #((fn [x key] {(key x) (dissoc x key)}) % name-key))
Copy link
Contributor

Choose a reason for hiding this comment

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

Тоже лямбда в лямбде. Достаточно оставить (fn [x] {(name-key x) (dissoc x name-key)})

(= x "5") (aggregate-and-filter keys-to-item-aggr item-req coll)
(= x "6") ["Good Bye\n"])
print-result)
(if (not= x "6") (main coll) nil)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Здесь when напрашивается вместо if

Comment on lines +210 to +216
(->> (cond
(= x "1") (select-data cust-keys :custID coll)
(= x "2") (select-data prod-keys :prodID coll)
(= x "3") (select-data sales-keys-to-show :salesID coll)
(= x "4") (aggregate-and-filter keys-to-cust-aggr cust-req coll)
(= x "5") (aggregate-and-filter keys-to-item-aggr item-req coll)
(= x "6") ["Good Bye\n"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Тут стоит case использовать вместо cond.

print-result)
(if (not= x "6") (main coll) nil)))

(main full-data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Не стоит использовать имя "main" для именования функций, которые не являются точками входа в программу, то есть не принимают строго аргументы командной строки.

Кроме того, производить какие-то побочные эффекты прямо в теле неймспейса - плохая практика. В данном случае следует объявить точку входа и настроить проект так, чтобы оная (точка входа) получала управление при выполнении команды lein run (нужно настроить :main-is).



;; собираем все данные в одну таблицу (список мап)
(def full-data (->> (map #(read-to-maps %1 %2) files key-names)
Copy link
Contributor

Choose a reason for hiding this comment

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

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

А ещё подобные программы перечитывают файлы при каждом запросе от пользователя, ведь общение с последним происходит в диалоговом режиме, а в процессе диалога данные на диске могут и измениться. Кроме того не стоит обращаться к диску за теми данными, которые пользователю не нужны: как только попросит, тогда и следует нужные файлы читать.

@astynax
Copy link
Contributor

astynax commented Jan 15, 2024

По домашней работе №6 оставил сколько-то замечаний, но они в известной степени стилистические, так что саму работу зачитываю.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants