[Clojure] Idiomatic Clojure - Code Smells (관용적인 클로저 - 코드 스멜)

2021 첫글은 역시 Clojure

프로그래밍 언어를 공부할때, 문법다음으로 공부해하는것이 Idiom 이다.

Idiom 을 아는것은, ~~ 스러운 코딩을 하는데 도움을 준다, (ex. python 스럽게 코딩하기, Java 를 Java 답게 코딩하기..)

그래서 Clojure/F# 에 대한 Idiom 을 찾아보던중 좋은 포스팅을 발견하였다.

Idiomatic Clojure: Code Smells (bsless.github.io)

Idiomatic Clojure: Code Smells

내용이 좋아서 한글로 정리하였음.

원작자 에게 번역하여 포스팅하는것을 허락받음

 

 

Mapping

매핑 결과를 합칠때에는 mapcat 을 사용해라

;;; Don't                       Do
(apply concat (map f xs))  =>  (mapcat f xs)

 

예를 들어

user=> (defn f [x] (* x x))
#'user/f
user=> (def xs [[1 2 3] [11 22 33][111 222 333]] )
#'user/xs

;; 아래 리스트를 합치고 싶다면
user=> (map reverse xs)
((3 2 1) (33 22 11) (333 222 111))

;; 이렇게 하지말고
user=> (apply concat (map reverse xs))
(3 2 1 33 22 11 333 222 111)

;; 이렇게
user=> (mapcat reverse xs)
(3 2 1 33 22 11 333 222 111)

관용적인 for를 쓰지말것

;;; Don't                Do
(for [x xs] (f x))  =>  (map f xs)

예를 들어

user=> (defn f [x] (* x x))
#'user/f
user=> (def xs [1 2 3 4])
#'user/xs
user=> (for [x xs] (f x))
(1 4 9 16)
user=> (map f xs)
(1 4 9 16)

위 두개를 합치는건 진짜 하지마라

;;; Please Don't
(apply concat (for [x xs] (f x)))

람다를 쓰지 말것

람다로 감쌀 필요가 없으면 감싸지 말것

;;; Don't             Do
(map #(f %) xs)  =>  (map f xs)

컬랙션내에 연관 데이터를 원할때, 이를 실체화 하라 (?)

컬랙션 내에서, 특정 키에 특정 값이 있는지 확인하는경우, 컬렉션의 순차적인 특징보다는, 컬렉션이 가지고 있는 연관성 (Associative) 를 원하는 경우이다.

(def people
  [{:person/name "Fred"}
   {:person/name "Ethel"}
   {:person/name "Lucy"}])

(defn person-in-people?
  [person people]
  (some #(= person (:person/name %)) people))

(person-in-people? "Fred" people);; => true
(person-in-people? "Bob" people) ;; => nil

;;; -> 이름을 찾는데, people 리스트가 순차적인것은 의미가 없음
;;; 원하는 것은 people 에 "Fred" 이름의 사람이 있는지 (연관성) 임

따라서 컬렉션을 원하는 데이터로 인덱싱 하여 사용해라

;; Group-by 로 인덱싱
(def collected-people
  (group-by :person/name people))

(contains? collected-people "Fred");; => true
(contains? collected-people "Bob") ;; => false


;; set/index 로 인덱싱
(def collected-people
  (clojure.set/index people [:person/name]))

(contains? collected-people {:person/name "Fred"});; => true
(contains? collected-people {:person/name "Bob"}) ;; => false

위 코드는 자연스럽게 읽힌다.

특정 속성에 대한 필터링이 반복적으로 이루어지는 경우, 해당 속성을 group-by 나 set/index 로 인덱싱 할수있다.

함수로 인덱싱하기 할때는 group-by를 키값으로 인덱싱 하기 위해서는 set/index 사용하라.

단 set/index의 경우 인덱싱하기 위한 요소가 set 일때만 사용해야한다.

 

필터링

filter <-> remove

pred을 not 으로 감싸지 말고, 그냥 반대되는 함수를 사용할것

;;; Don't                      Do
(filter #(not (p %)) xs)  =>  (remove p xs)
(remove #(not (p %)) xs)  =>  (filter p xs)

보다 간결한 탐색 함수 사용

;;; Don't               Do
(first (first xs)) =>  (ffirst xs)
(first (next xs))  =>  (fnext xs) or (second xs)
(next (first xs))  =>  (nfirst xs)
(next (next xs))   =>  (nnext xs)

예시

user=> (def xs [[1 2 3] [11 22 33] [111 222 333]])
#'user/xs
user=> (first (first xs))
1
user=> (ffirst xs)
1

user=> (first (next xs))
[11 22 33]
user=> (fnext xs)
[11 22 33]
user=> (second xs)
[11 22 33]

user=> (next (first xs))
(2 3)
user=> (nfirst xs)
(2 3)

user=> (next (next xs))
([111 222 333])
user=> (nnext xs)
([111 222 333])

 

공백 (Emptiness)

empty? 의 정의는 다음과 같다.

(defn empty?
  "Returns true if coll has no items - same as (not (seq coll)).
  Please use the idiom (seq x) rather than (not (empty? x))"
  [coll] (not (seq coll)))

그러니까 empty? 는 (not (seq coll)) 요거랑 같고 (not (empty? x)) 대신 제발 (seq x) 를 써달라는 내용..

;;; Don't                        Do
(when (not (empty? x)) ...)  => (when (seq x) ...)
(when-not (empty? x) ...)    => (when (seq x) ...)
(when (= 0 (count x)) ...)   => (when (empty? x) ...)
(when (< 0 (count x)) ...)   => (when (seq x) ...)

참고로 seq 를 doc 으로 확인해보면 다음과 같다.

user=> (doc seq)
-------------------------
clojure.core/seq
([coll])
  Returns a seq on the collection. If the collection is
    empty, returns nil.  (seq nil) returns nil. seq also works on
    Strings, native Java arrays (of reference types) and any objects
    that implement Iterable. Note that seqs cache values, thus seq
    should not be used on any Iterable whose iterator repeatedly
    returns the same mutable object.

설명에 나와있듯이 (seq nil) 이나 (seq '()) -> nil, (seq '(1 2 3)) -> (1 2 3)

 

Into

Into 가 종종 잘못 사용되는 Case 가 3가지 있다.

시퀀스 타입 변환을 위해서 into를 사용하지 말것

;;; Don't           Do
(into [] xs)   =>  (vec xs)
(into #{} xs)  =>  (set xs)

예시

user=> xs
(1 2 3 4)
user=> (into [] xs)
[1 2 3 4]
user=> (vec xs)
[1 2 3 4]

맵 매핑을위해 사용하지 말것

;;; Don't
(into {} (map (fn [[k v]] [k (f v)]) m))
(into {} (for [[k v] m] [k (f v)]))
;;; Do
(reduce-kv (fn [m k v] (assoc m k (f v))) {} m)
;;; Or faster* but less pretty
(persistent!
 (reduce-kv (fn [m k v] (assoc! m k (f v))) (transient {}) m))

예시

user=> m
{:a 1, :b 2, :c 3}
user=> (into {} (map (fn [[k v]] [k (f v)]) m))
{:a 1, :b 4, :c 9}
user=> (reduce-kv (fn [m k v] (assoc m k (f v))) {} m)
{:a 1, :b 4, :c 9}

Transducer API 를 사용할것

;;; Don't                       Do
(into coll (map f xs))     =>  (into coll (map f) xs)
(into coll (filter p xs))  =>  (into coll (filter p) xs)

 

Map 작업

Map 에서 nil

Clojure 에서 map 은 Slot 이 아니라 컬렉션. (없으면 없는거지 정해진 공간에 값을 채우는 개념이 아님)

따라서 되도록 map 에 nil을 넣지 말것.

아래 코드를 이용하여 map 의 nil을 제거할수있음.

(defn assoc-some [m k v] (if (nil? v) m (assoc m k v)))
(defn prune-nils [m] (reduce-kv assoc-some {} m))

(prune-nils {:a 1 :b nil :c 2 :d nil}) ;; => {:a 1, :c 2}

또는 아래 코드를 이용

(defn remove-nil [m k v] (if (nil? v) (dissoc m k) m))
(defn prune-nils [m] (reduce-kv remove-nil m m))

성능이 중요한 경우, merge 를 사용할때 주의하며 사용할것

merge 는 성능이 그다지 좋지 않음

성능에 대한 내용에 관심이 있다면 Naked Performance (with Clojure) – Tommi Reiman - YouTube 강연을 시청

다음 섹션을 읽은뒤, 성능을 고려하여 merge 를 사용할것.

빠른 merge 를 위해서는 GitHub - bsless/clj-fast: Unpredictably faster Clojure 을 사용할수는 있지만, 일반적인 경우는 기본 merge 함수를 사용하는것이 좋음

직접 머지하는것을 피할것

(let [m1 {}
      m1 (if (nil? (:k m2)) m1 (assoc m1 :k (:k m2)))])

위 코드는 , m2 의 :k 항목이 nil이 아니면 m2의 :k 항목 값을 m1 에 추가하는 코드이다.
m2 에서 m1으로 추가할 key가 엄청 많아진다면 코드는 더 복잡하지고, 나중에 해당 코드를 이해하기 어려워질것이다.

따라서 아래와 같은 코드를 사용하는것이 좋다.

;prune-nils 는 'Map 에서 nil' 절 에서 정의된 함수
(merge m1 (prune-nils m2))

직접 키를 선택하는것을 피할것

merge 된 map 을 만들기 위하여 다수의 map 에서 수동으로 key를 뽑아서 새 Map 을 만드는것이 가능하다.

{:a (:a m1)
 :b (:b m2)}

바로 직전 절에서와 마찬가지로 합쳐할 Map이 크고 많다면 코드가 복잡해지고 나중에 이해하기 어렵다.

아래 코드를 이용하는것이 좋다

(merge (select-keys m1 [:a])
       (select-keys m2 [:b]))

조건에 따른 맵 빌드업

아래와 같은 패턴이 종종 사용된다.

(defn foo
  [in]
  (let [m {:k0 (f0 in)} ;; mandatory
        m (if (p1 in) (assoc m :k1 (f1 in)) m) ;; optional
        m (if (p2 in) (assoc m :k2 (f2 in)) m)]
    m))

let 을 이용하여 m 에 조건부로 항목을 추가한다. (p1 조건 만족시 :k1 항목 추가됨, p1,p2 조건 만족시 :k2항목 추가됨 ...)

이럴때에는 cond-> 를 이용하자

(defn foo
  [in]
  (cond-> {:k0 (f0 in)} ;; mandatory
    (p1 in) (assoc :k1 (f1 in)) ;; optional
    (p2 in) (assoc :k2 (f2 in))))

cond-> 를 사용하면, 흐름제어(if 문 flow)가 구문(syntax) 으로 변경되며, 추적할 상태가 없어진다.

 

숫자

clojure 에는 숫자를 위한 기능이 제공된다.

0? 양수? 음수?

;;; Don't     Do
(= 0 x)  =>  (zero? x)
(> x 0)  =>  (pos? x)
(< x 0)  =>  (neg? x)
에 관한 작은 경고 : 그것은 

zero? pos? neg? 함수를 사용하자

1 더하기 빼기 (i++ / i--)

;;; Don't     Do
(+ 1 x)  =>  (inc x)
(- 1 x)  =>  (dec x)

1을 더할땐 inc / 1을 뺄때는 dec 를 사용하자

true / false / nil 확인

;;; Don't          Do
(= true x)   =>   (true? x)
(= false x)  =>   (false? x)
(= nil x)    =>   (nil? x)

true / false / nil 을 확인하기 위하여 비교함수(=) 를 사용할 필요가 없다.

 

doall

doall 은 Lazy 시퀀스를 강제적으로 realize 하는 함수이다.
doall 을 생산에 사용하면 안된다.

doall 에 대한 대안으로는
doall in a single threaded
doall in a multi-threaded 참고

아래 코드는 잘못된 doall 에 대한 잘못된 코드이다,

(doall (doseq [x xs] ..))

1. doseq 는 이미 엄격(strict)함
2. doseq 는 nil을 리턴함

 

Style

암시적인 do 블록

일부 표현식에는 do 가 암시적으로 포함되어있기 때문에, do 를 명시적으로 사용할 필요가 없다.

when

;;; Don't
(when test
  (do expr1
      expr2))

;;; Do
(when test
  expr1
  expr2)

let

;;; Don't
(let bindings
  (do expr1
      expr2))

;;; Do
(let bindings
  expr1
  expr2)

function body

;;; Don't
(fn []
  (do expr1
      expr2))

;;; Do
(fn []
  expr1
  expr2)

when / let / function body 말고도 try,catch 이나 & body 타입 시그니쳐를 가진 메크로 등에서는 do를 명시적으로 사용할 필요가 없음.

 

쓰레딩 (파이프라인)

의미없는 쓰레딩을 피하라

;Don't           Do
(-> x f)     => (f x)
(-> x (f a)) => (f x a)

컬랙션을 위한 -> / 시퀀스를 위한 ->>

Threading with Style 참고

함수를 계속 호출하는것(deep call stack) 보다는 쓰레드 메크로를 사용해라

;;; Don't
(defn h [x] ...)
(defn g [x] ... (h x))
(defn f [x] ... (g x))

;;; Do
(-> x f g h)

some-> 쓰레드 메크로를 사용해라

when-let 을 중첩하는것 보다는 some-> 쓰레드 메크로를 사용해라
let 에서 if문으로 nil 을 확인하는것 보다는 some-> 쓰레드 메크로를 사용해라

;;; Don't
(when-let [x1 (f0 x0)]
  (when-let [x2 (f1 x1)]
    (when-let [x3 (f2 x2)]
      (f3 x3))))

;;; Do
(some-> x0 f0 f1 f2 f3)

;;; Don't
(let [x (if (nil? x) nil (x f0))
      x (if (nil? x) nil (x f1))
      x (if (nil? x) nil (x f2))]
  (if (nil? x) nil (f3 x)))

;;; Do
(some-> x0 f0 f1 f2 f3)

cond-> 쓰레드 메크로를 사용해라

(let [x (if (p0 x) (f0 x) x)
      x (if (p1 x) (f1 x) x)]
  (if (p2 x) (f2 x) x))

(cond-> x
  (p0 x) f0
  (p1 x) f1
  (p2 x) f2)

단 cond-> 를 사용하는경우 조건들(p0, p1, p2 ...) 이 사용하는 x 값이 이전 계산결과에 따라 변경되지 않는다는 차이점이 존재한다.
(예를들어, (p0 x) 의 결과가 (p1 x) 를 판단하는데 영항을 주지 않음)

이전 계산 결과를 반영하도록 하기 위해서 아래 매크로를 이용할수 있다.

(defmacro cond*->
  [expr & clauses]
  (assert (even? (count clauses)))
  (let [g (gensym)
        steps (map (fn [[test step]] `(if (-> ~g ~test) (-> ~g ~step) ~g))
                   (partition 2 clauses))]
    `(let [~g ~expr
           ~@(interleave (repeat g) (butlast steps))]
       ~(if (empty? steps)
          g
          (last steps)))))

(cond*-> x
         p0 f0
         p1 f1
         p2 f2)

무의미한 let 제거 (let Shadowing 제거)

대부분의 경우 let 안에서 값을 refer 할 필요가없음

(let [x (f0 x)
      x (f1 x)
      x (f2 x)]
  (f3 x))

보다는

(-> x f0 f1 f2 f3)

가 좋음

중첩제거

바인딩 폼이있는 메크로들은 대게 중첩할 필요가 없다.

;;; Don't
(let [x 1]
  (let [y 2]
    [x y]))

;; Do
(let [x 1
      y 2]
  [x y])

중첩된 이터레이션의 중첩도 제거할수있다.

(doseq [x xs]
  (doseq [y ys]
    (println x y)))

;;; Do
(doseq [x xs
        y ys]
  (println x y))

매핑 레퍼

아래는 do-one-thing 함수와 이를 반복하는 do-one-thing-n-times 함수의 예시이다.

(defn build-person [x] ...)
(defn build-persons [xs] (map build-person xs))

위와 같은 코드는 잘 느껴지지않지만, 다음과 같은 이유로 코드 스멜이다.

1. 구현(Implementation)과 구체화(Concretion)가 연결되어있음 : 하나(person) 가 아닌 시퀀스(persons) 를 처리하는 상황에 있게 되어, 하나(person) 과 시퀀스(persons) 에서 왔다갔다 해야함. (?? 먼소리지)
2. 깨지기 쉬움: build-persons 에 로직이 추가되면, 단일 함수(build-person) 만 사용하는 시스템에는 반영되지 않음
3. 나쁘게 합성됨 : 쓰레드 메크로나 comp를 사용하여 여러개의 단일함수를 합성하거나, 트랜스듀서를 이용하여 순차적인 경우를 합성할수 있지만, 해당 구현은 구체적으로 합성되어있음.
4. 낮은 재사용성 : 설명이 무슨 뜻인지 모르겠음..

-> 이 섹션의 내용이 솔직히 이해되지 않아서, Facebook Clojure Korea Group 의 답변을 추가로 달아둠 (김영태님께서 답변해주셧습니다)

이원준님이 질문하신 내용에 답변드립니다.

;;; Don't

;; 정의

(defn build-person [x] ...)

(defn build-persons [xs] (map build-person xs)) ; (1)

;; 호출

(build-persons xs)

;;; Do

;; 정의

(defn build-person [x] ...)

;; 호출

(map build-person xs) ; (2)

(map (comp build-person ,,,) xs) ; (3)

(transduce (comp (map build-person) ,,,) conj xs) ; (4)

결론적으로 글쓴이가 전하고자 하는 요지는, 단수명 함수(build-person)를 다시 복수명 함수(build-persons)로 감싸지 말라는 것입니다(물론 이것은 일반론이고, 코드 상에서 build-persons 함수를 수백번 호출해아만 하는 상황에서는 (1)의 구현이 더 나을 수도 있습니다).

(1)은 (2)라는 구체적인 사례(concretion)에만 적용되는 함수일 뿐, (3)이나 (4)의 경우에는 적용할 수 없기 때문입니다.

 

인자의 위치

인자 개수를 조절할것 (입력 인자값의 폭발을 피하라)

함수가 매우 많은 수의 인자값을 사용하면, 여러 부분으로 쪼개는것을 고려해라, 모든 인자가 한번에 사용하는 경우는 거의 없다,

;;; Don't
(defn f
  [fizz buzz foo bar quux ohgod enough])

Key-Value 나머지 인자에서 Map 을 사용하는것을 고려하라

;;; 일반적인 나머지 인자 (rest) 사용했을때
(defn f
  ([x y]
   (f x y :a 1 :b 2))
  ([x y & {a :a b :b}]
   ,,,))

;;사용예
(f x y :a 3 :b 4)


;;; Map 을 인자로 사용했을때
(defn f
  ([x y]
   (f x y {:a 1 :b 2}))
  ([x y {a :a b :b}]
  ,,,))

;;사용예
(f x y {:a 3 :b 4})

함수를 고차 함수로 전달하고자 할 때 꼬리 인수의 유형을 염두에두고 항상 Unpack 해야하며, 일반적으로 모든 곳에 apply 를 사용해야함을 기억해야하지만, Map 을 인자로 사용하면 사용하면 쉽게 피해갈수 있음.

반환값이 여러개 일때, 위치를 이용하기 보다 Map 을 사용하라

비즈니스적인 의미를 명시적인 키워드로 나타낼 수 있는 Map 을 사용하는것이 좋음

;;; 위치를 이용했을때
(defn sieve
  [p xs]
  [(filter p xs) (remove p xs)])

;; first / second 등에서는 로직적인 의미가 존재하지 않음.
(first (sieve even? (range 9)))
;; => (0 2 4 6 8)


;;; Map 을 이용했을때
(defn sieve
  [p xs]
  {:true (filter p xs) :false (remove p xs)})

;; Filtered 된 요소가 :true, 그렇지 않은 요소가 :false 임이 보다 명시적으로 나타남.
(:true (sieve even? (range 9)))
;; => (0 2 4 6 8)

 

끝

 

 

PS1. 해당 블로그의 다른 Idiomatic Clojure 시리즈도 보는것을 추천
PS2. 번역을 잘하기 위해서는 양쪽 언어를 둘다 잘해야한다는것을 깨달았다. 뭐로 번역할지 감이 잘 안잡힘