Skip to content

Conversation

vadych
Copy link

@vadych vadych commented May 11, 2023

No description provided.

@@ -1,4 +1,14 @@
(ns otus-02.homework.palindrome)
(ns otus-02.homework.palindrome
(:require [clojure.string :as s]))
Copy link
Collaborator

@margintop15px margintop15px May 15, 2023

Choose a reason for hiding this comment

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

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

(defn common-child-length [first-string second-string]
(cond
(or (empty? first-string) (empty? second-string)) 0
(= (last first-string) (last second-string))
Copy link
Collaborator

Choose a reason for hiding this comment

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

хорошее решение
один вопрос - почему сравнение ведётся с конца строки используя last?
кажется что можно сделать тоже самое но начать с начала используя first в таком случае не прийдётся находить длину строки и вычитать единицу (- (count second-string) 1)

Copy link
Author

Choose a reason for hiding this comment

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

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

Comment on lines +7 to +8
(filter some?
(map alphabet
Copy link
Collaborator

Choose a reason for hiding this comment

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

хороший вариант с сетом и фильтрацией
еще есть вариант с использованием clojure.string/replace
(s/replace (s/lower-case s-not-normal) #"[^a-z]" "")
регулярка #"[^a-z]" сматчится на всё кроме букв

@@ -1,4 +1,9 @@
(ns otus-02.homework.pangram)
(ns otus-02.homework.pangram
(:require [otus-02.homework.palindrome :refer [normalize-str, alphabet]]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

запятые можно не ставить clojure их игнорирует, но если вам так удобнее то можно ставить

Copy link
Author

Choose a reason for hiding this comment

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

Это старая привычка - запятые :)

(defn is-pangram [test-string])

(defn is-pangram [test-string]
(= alphabet
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍



(defn encode-string [input])
(defn mix [v r]
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

Спасибо за это замечание. Я с Вами согласен, но старался следовать соглашению о стиле https://github.com/bbatsov/clojure-style-guide#idiomatic-names
Возможно я его приватно истолковал

(defn encode-string [input]
(let [n-input (normalize-str input)
[r _] (size-field (count n-input))
v-input (partition r r (cycle " ") n-input)
Copy link
Collaborator

Choose a reason for hiding this comment

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

отлично что использовали cycle

Copy link
Author

Choose a reason for hiding this comment

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

Буду честен, просто утянул с интернета. Мне нужна была бесконечная последовательность пробелов. :)

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