Делаем Clojure более функциональным

Question

В моем первом проекте Clojure все получилось хорошо, за исключением этой части в конце:

(let [broken-signs    (->> (:symbols field)
                           (map make-sign)
                           (filter broken?))
      broken-count    (count broken-signs)
      unfixable-count (-> (filter (complement fixable?) broken-signs)
                          (count))]
  (println
    (if (> unfixable-count 0)
      -1
      (- broken-count unfixable-count))))

Отступы выглядят отключенными и не чувствуют себя функциональными, так как я повторно использую состояние в блоке let.Я в основном считаю количество сломанных знаков, а затем количество исправляемых знаков.Если какой-либо знак является нефиксированным, я печатаю -1, в противном случае я печатаю количество знаков, которые нужно исправить.

Если бы я отобразил / отфильтровал дважды, у меня был бы дублирующий код, но больше всего он работал быпомедленнее.Тем не менее, есть ли способ улучшить этот код?

РЕДАКТИРОВАТЬ: Это то, что я остановился на

 (defn count-broken-yet-fixable []
   (let [broken (->> (:symbols field)
                     (map make-sign)
                     (filter broken?))
         unfixable (remove fixable? broken)]
     (when (empty? unfixable)
       (count broken))))

 (defn solve-task []
   (if-let [result (count-broken-yet-fixable)]
     result
     -1))

(println (solve-task))

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

Answer 1

Я не думаю, что в вашем подходе есть что-то "не функциональное" или неправильное.Отступы выглядят хорошо.

(let [broken (->> (:symbols field)
                  (map make-sign)
                  (filter broken?))
      unfixable (remove fixable? broken)]
  (when (seq unfixable)
    (- (count broken) (count unfixable))))

• Вы можете заменить filter (complement на remove
• Можно использовать pos? вместо (> n 0)
• Iможет поставить два println внутри if, но на самом деле лучше вернуть значение
• Вы можете встроить привязку broken-count, поскольку она используется только в одном месте
• Iлично я думаю, что это легче читать с меньшим количеством макрокоманд
• Поскольку необходимость подсчета unfixable s является условной, вы можете проверить значения с seq first
• Если вы вернете -1 в качестве часового значения я бы использовал nil вместо этого;это происходит естественным образом, когда условие when не выполняется
• Условная логика кажется обратной: вы возвращаете -1, когда unfixable-count положительно, и используете его значение только тогда, когда оно не положительно (что означает, что ононулевой счет b / c не может быть отрицательным), например, он может быть переписан как (- broken-count 0), а затем просто broken-count

Answer 2

(let [broken-signs (->> (:symbols field)
                        (map make-sign)
                        (filter broken?))]
  (if-let [unfixable-signs (seq (remove fixable? broken-signs))]
    -1
    (- (count broken-signs) (count unfixable-signs)))

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

Возвращаемое значение может затем использовать print для выполнения фактических действий.

Answer 3

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

                      ; align here (controversial)
(let [broken-signs    (->> (:symbols field)
                           (map make-sign)
                           (filter broken?))
      broken-count    (count broken-signs)
      unfixable-count (->> broken-signs ; maybe emphasize non-function start
                           (filter (complement fixable?))
                           count)] ; no parens needed
  ;; don't print; just return number
  (if (< 0 unfixable-count)  ; prefer less-than (Elements of Clojure book)
    -1
    (- broken-count unfixable-count)))

Я настоятельно рекомендую Руководство по стилю Clojure для соответствующих предложений.