Skip to content

Conversation

MitrickX
Copy link

@MitrickX MitrickX commented May 9, 2023

Necessary minimum:

  1. Palindrom
  2. Pangram
  3. Common child length
  4. Square code

Comment on lines 30 to 38
(if (= x y)
(recur
(rest y-seq)
(rest row)
(conj next-row (inc top-left)))
(recur
(rest y-seq)
(rest row)
(conj next-row (max top left))))))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Здесь в обеих ветках используется практически одинаковый вызов recur. Кажется, можно вынести отличающуюся часть третьего аргумента вместе с условием в вышестоящий let и тогда останется один вызов recur.

Copy link
Author

Choose a reason for hiding this comment

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

Могу поправить, если нужно, надо исправлять?

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.

fixed

@MitrickX MitrickX changed the title Homework2 - necessary minimum Homework2 May 10, 2023
(defn common-child-length [fist-string second-string])
;; Хелпер, который строит следующую строку dynamic programming таблицы по текущей
(defn next-row-helper [x y-seq row]
(loop [y-seq y-seq
Copy link
Collaborator

Choose a reason for hiding this comment

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

хорошее решение
есть один совет по стилю. это не ошибка, но иногда приводит в замешательство
y-seq у вас и имя аргумента и имя переменной в цикле
это еще называется variable shadowing - https://en.wikipedia.org/wiki/Variable_shadowing
во многих компаниях это считается не очень хорошим стилем и программисты стараются этого избегать

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-palindrome [test-string]
(let [original (str/lower-case
(str/replace test-string #",|!|\?|\.|\s" ""))
Copy link
Collaborator

Choose a reason for hiding this comment

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

хорошее решение
в этом месте регулярку можно немного упростить
(s/replace (s/lower-case s-not-normal) #"[^a-z]" "")
#"[^a-z]" сматчится на всё кроме букв

Copy link
Author

Choose a reason for hiding this comment

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

Согласен, я переусложнил. Фиксить не вижу смысла сейчас)

;; Чтение стобцов слева-направо в строку
(defn read-rectangle [rectangle]
(apply str
(loop [result '()
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.

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

Comment on lines +71 to +72
complete-slices (partition-all cols-cnt complete-sub-str)
incomplete-slices (partition-all (dec cols-cnt) incomplete-sub-str)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут можно немного упростить если использовать partition
у этой функции есть специальный параметр pad (partition n step pad coll) элементы которого будут использоваться если не хватает элементов для равных слайсов

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.

3 participants