Статьи

Clojure: как предотвратить «ожидаемую карту, полученный вектор» и подобные ошибки

Февраль 22, 2019

То, что мой код Clojure делает большую часть времени, это преобразование данных. Тем не менее, я не вижу формы преобразования данных — я должен знать, как эти данные выглядят на входе, и держать мысленную модель их изменения на каждом этапе. Но я делаю ошибки. Я делаю ошибки в своем коде, чтобы данные больше не соответствовали модели, которой они должны следовать. И я допускаю ошибки в своей ментальной модели того, как выглядят данные в настоящее время, что может привести к ошибке кода в дальнейшем. Конечный результат тот же — небольшое полезное исключение на более позднем этапе, касающееся неправильной формы данных. Здесь есть две проблемы: Ошибка обычно предоставляет слишком мало полезной информации, и она обычно проявляется позже, чем на самом деле ошибка кода / модели. Поэтому я легко потратил час или больше на устранение этих ошибок. В дополнение к этому, трудно также прочитать такой код, потому что читателю не хватает умственной модели данных автора, и он сам должен извлечь ее — что довольно сложно, особенно если форма входных данных неясна с самого начала ,

Я должен отметить, что я, конечно, пишу тесты и экспериментирую в REPL, но я все еще сталкиваюсь с этими проблемами, поэтому этого мне недостаточно. Тесты не могут защитить меня от неправильной модели входных данных (поскольку я пишу [модульные] тесты, основанные на тех же предположениях, что и код, и обнаруживаю ошибку, только когда я интегрирую все биты), и даже если они помогают обнаружить ошибка, это все еще занимает много времени основной причиной.

Могу ли я сделать лучше? Я верю что смогу.

Трудно выявить ошибки с задержкой манифестации и сложный для понимания код, который сообщает только половину истории (преобразования, но не форму преобразуемых данных) — это цена, которую мы платим за силу динамической типизации. Но есть стратегии по снижению этой цены. Я хочу представить три из них: небольшие, сфокусированные функции с хорошими именами, деструктурирование в качестве документации и разумное использование предварительных и постусловий.

Содержание этого поста основано на том, что я узнал от Улиса Червиньо Берези во время одного из бесплатных часов в офисе Clojure, которые он щедро предлагает , подобно Лейфу в США.

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

Основная идея:

  1. Разбейте преобразования на маленькие простые функции с понятными именами
  2. Используйте деструктурирование в аргументах функции для документирования ожидаемых данных.
  3. Используйте предварительные и последующие условия (и / или утверждения) как проверки, так и документацию
  4. (Все тесты и интерактивные исследования в REPL, которые вы уже делаете.)

Упрощенный пример

У нас есть интернет-магазин, который продает автомобили со скидкой. У нас также есть периодические акции с повышенными скидками на отдельные автомобили. Для каждого автомобиля у нас также есть несколько ключевых слов, которые люди могут использовать, чтобы найти его и категории, к которым он принадлежит. Ниже приведен код, который обрабатывает необработанные данные о автомобилях + кампаниях + поиске по ключевым словам из запроса БД: сначала оригинал, а затем рефакторинг с проверками:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
(require '[cheshire.core :refer [parse-string]])
 
(defn- json->data [key m]
  (update-in m [key] #(parse-string % true)))
   
(defn- select-campaign [car+campaigns]
  (first car+campaigns))
 
(defn- jdbc-array-to-set
  [key m]
  (update-in m [key] #(apply hash-set (.getArray %))))
 
(defn refine-cars-simple
  "Process get-cars query result set - derive additional data, transform values into better ones
   There is one row per car and campaign, a car may have more campaigns - we pick the best one.
  "
  [cars-raw]
  (->>
   cars-raw
   (map (partial json->data :json)) ;; <- this I originally forgot
   ;; group rows for the same car => [[car1 ..][car2 ..]]
   (group-by :id)
   (vals)
   ;; join all car1 vectors into one car ..
   (map select-campaign)
   (map (fn [car]
          (->>
           car
           (jdbc-array-to-set :category_ref)
           (jdbc-array-to-set :keywords))))))
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
(require '[cheshire.core :refer [parse-string]])
(require '])
 
(defn- car? [{:keys [id] :as car}]
  (and (map? car) id))
 
(defn- json->data [key m]
  {:pre [(contains? m key) (string? (get m key))], :post [(map? (get % key))]}
  (update-in m [key] parse-string true))
   
(defn- select-campaign [[first-car :as all]]
  {:pre [(sequential? all) (car? first-car)], :post [(car? %)]}
  first-car)
 
(defn- jdbc-array-to-set
  [key m]
  {:pre [(contains? m key) (instance? java.sql.Array (get m key))], :post [(set? (get % key))]}
  (update-in m [key] #(apply hash-set (.getArray %))))
 
(defn group-rows-by-car [cars-raw]
  {:pre [(sequential? cars-raw) (car? (first cars-raw))]
   :post [(sequential? %) (vector? (first %))]}
  (vals (group-by :id cars-raw)))
 
(defn refine-car [car]
  {:pre [(car? car) (:keywords car) (:category_ref car)]}
  (->> car
    (jdbc-array-to-set :category_ref)
    (jdbc-array-to-set :keywords)))
 
(defn refine-cars-simple
  "Process get-cars query result set - derive additional data, transform values into better ones
   There is one row per car and campaign, a car may have more campaigns - we pick the best one.
  "
  [cars-raw]
  (->>
   cars-raw
   (map (partial json->data :json)) ;; <- this I originally forgot
   (group-rows-by-car)
   (map select-campaign)
   (map refine-car)))
 
(defn empty-array [] (reify java.sql.Array (getArray [_] (object-array []))))
 
(refine-cars-simple [{:id 1, :json "{\"discount\":5000}", :campaign_discount 3000, :category_ref (empty-array), :keywords (empty-array)}])

Пример из реального мира

У нас есть интернет-магазин, который продает автомобили со скидкой. У каждого автомобиля, который мы продаем, есть базовая скидка (абсолютная сумма или процент), и мы также проводим периодические кампании для выбранных автомобилей. Для каждого автомобиля у нас также есть несколько ключевых слов, которые люди могут использовать, чтобы найти его.

Оригинальный код

Ниже приведен код, который обрабатывает необработанные данные о автомобилях + кампаниях + поиске по ключевым словам из запроса БД, выбирает наиболее подходящую кампанию и рассчитывает окончательную скидку:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
(require '[cheshire.core :refer [parse-string]])
 
(defn db-array [col] (reify java.sql.Array (getArray [_] (object-array col))))
 
(defn- json->data [data fields]
  (map (fn [data]
         (reduce (fn [data field]
                   (assoc data field (parse-string (get data field) true)))
                 data fields)) data))
 
(defn- discount-size [discount]
  (if (or
       (> (:amount discount) 10000)
       (> (:percent discount) 5))
    :high
    :normal))
 
(defn- jdbc-array-to-set
  "Convert a PostgreSQL JDBC4Array inside the map `m` - at the key `k` - into a se"
  [key m] (update-in m [key] #(some->> % (.getArray) (into #{}))))
 
(defn- compute-discount
  "Derive the :discount map based on the car's own discount and its active campaign, if applicable"
  [car]
  (let [{:keys [json campaign_discount_amount campaign_discount_percent]} car
        {:keys [discount_amount discount_percent]} json
        discount? (:use_campaign car)
        amount (if discount?
                 (apply + (remove nil? [discount_amount campaign_discount_amount]))
                 discount_amount)
        percent (if discount?
                  (apply + (remove nil? [discount_percent campaign_discount_percent]))
                  discount_percent)
        discount {:amount amount
                  :percent percent}
        discount-size (discount-size discount)
        ]
    (assoc car :discount discount :discount-size discount-size)))
 
(defn merge-campaigns
  "Given vector of car+campaign for a particular car, return a single car map
   with a selected campaign.
  "
  [car+campaigns]
  {:pre [(sequential? car+campaigns) :id]}
  (let [campaign-ks [:use_campaign :campaign_discount_amount :campaign_discount_percent :active]
        car (apply dissoc
                    (first car+campaigns)
                    campaign-ks)
        campaign (->> car+campaigns
                      (map #(select-keys % campaign-ks))
                      (filter :active)
                      (sort-by :use_campaign) ;; true, if any, will be last
                      last)]
    (assoc car :best-campaign campaign)))
 
(defn refine-cars
  "Process get-cars query result set - derive additional data, transform values into better ones
   There is one row per car and campaign, a car may have more campaigns - we pick the best one.
  "
  [cars-raw]
  (->> cars-raw
   (#(json->data % [:json]))
   (#(map merge-campaigns
        (vals (group-by :id %))))
   (map (comp            ;; reminder - the 1st fn is executed last
         compute-discount
         (fn [m] (update-in m [:keywords] (partial remove nil?))) ;; {NULL} => []
         (partial jdbc-array-to-set :keywords)
         (partial jdbc-array-to-set :category_ref)
         ))
   ))
 
(refine-cars [
              {:id 1
               :json "{\"discount_amount\":9000,\"discount_percent\":0}"
               :campaign_discount_amount 2000
               :campaign_discount_percent nil
               :use_campaign false
               :active true
               :keywords (db-array ["fast"])
               :category_ref (db-array [])}])

Дефекты и я

Первоначально у меня было две [обнаруженные] ошибки в коде, и обе заняли у меня много времени, чтобы исправить — сначала я забыл преобразовать JSON из строки в карту (неверное предположение о входных данных), а затем я запускаю кампании слияния непосредственно в списке списков автомобилей + кампаний вместо их отображения (предварительное условие ? не помогло обнаружить эту ошибку). Таким образом, преобразования явно слишком подвержены ошибкам.

Трассировки стека не содержали достаточно полезной контекстной информации (хотя более опытный Clojurist наверняка обнаружил бы и исправил коренные причины гораздо быстрее):

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
## Forgotten ->json:
java.lang.NullPointerException:
 clojure.lang.Numbers.ops Numbers.java:  961
  clojure.lang.Numbers.gt Numbers.java:  227
  clojure.lang.Numbers.gt Numbers.java: 3787
       core/discount-size     cars.clj:   13
    core/compute-discount     cars.clj:   36
-------------
## Forgotten (map ..):
java.lang.ClassCastException: clojure.lang.PersistentVector cannot be cast to clojure.lang.IPersistentMap
              RT.java:758 clojure.lang.RT.dissoc
            core.clj:1434 clojure.core/dissoc
            core.clj:1436 clojure.core/dissoc
          RestFn.java:142 clojure.lang.RestFn.applyTo
             core.clj:626 clojure.core/apply
cars.clj:36 merge-campaigns
...

рефакторинга

Это код, реорганизованный в более мелкие функции с проверками (и, конечно, его можно значительно улучшить):

001
002
003
004
005
006
007
008
009
010
011
012
013
014
015
016
017
018
019
020
021
022
023
024
025
026
027
028
029
030
031
032
033
034
035
036
037
038
039
040
041
042
043
044
045
046
047
048
049
050
051
052
053
054
055
056
057
058
059
060
061
062
063
064
065
066
067
068
069
070
071
072
073
074
075
076
077
078
079
080
081
082
083
084
085
086
087
088
089
090
091
092
093
094
095
096
097
098
099
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
(require '[cheshire.core :refer [parse-string]])
(require '])
 
(defn db-array [col] (reify java.sql.Array (getArray [_] (object-array col))))
 
(defn- json->data [data fields]
  {:pre [(sequential? data) (sequential? fields)]}
  (map (fn [data]
         (reduce (fn to-json [data field]
                   {:pre [(map? data) (string? (get data field)) (keyword? field)]}
                   (assoc data field (parse-string (get data field) true)))
                 data fields)) data))
 
(defn- discount-size [{:keys [amount percent] :as discount}]
  {:pre [(number? amount) (number? percent) (<= 0 amount) (<= 0 percent 100)]
   :post [(#{:high :normal} %)]}
  (if (or
       (> amount 10000)
       (> percent 5))
    :high
    :normal))
 
(defn- jdbc-array-to-set
  "Convert a PostgreSQL JDBC4Array inside the map `m` - at the key `k` - into a se"
  [key m]
  {:pre [(keyword? key) (map? m) (let [a (key m)] (or (nil? a) (instance? java.sql.Array a)))]}
  (update-in m [key] #(some->> % (.getArray) (into #{}))))
 
(defn car? [{:keys [id] :as car}]
  (and (map? car) id))
 
(defn- compute-discount
  "Derive the :discount map based on the car's own discount and its active campaign, if applicable"
  [{{:keys [discount_amount discount_percent] :as json} :json
    :keys [campaign_discount_amount campaign_discount_percent] :as car}]
  {:pre [(car? car) (map? json) (number? discount_amount) (number? discount_percent)]
   :post [(:discount %) (:discount-size %)]}
  (let [discount? (:use_campaign car)
        amount (if discount?
                 (apply + (remove nil? [discount_amount campaign_discount_amount]))
                 discount_amount)
        percent (if discount?
                  (apply + (remove nil? [discount_percent campaign_discount_percent]))
                  discount_percent)
        discount {:amount amount
                  :percent percent}
        discount-size (discount-size discount)
        ]
    (assoc car :discount discount :discount-size discount-size)))
 
(defn select-campaign
  "Return a single car map with a selected campaign."
  [{:keys [campaigns] :as car}]
  {:pre [(car? car) (sequential? campaigns)]
   :post [(contains? % :best-campaign)]}
  (let [best-campaign (->> campaigns
                          (filter :active)
                          (sort-by :use_campaign) ;; true, if any, will be last
                          last)]
    (-> car
        (dissoc :campaigns)
        (assoc :best-campaign best-campaign))))
 
(defn nest-campaign [car]
  ;; :pre check for campaing keys would require too much repetition => an assert instead
  {:pre [(car? car)]
   :post [((comp map? :campaign) %)]}
  (let [ks (set (keys car))
        campaign-ks #{:campaign_discount_amount :campaign_discount_percent :use_campaign :active}
        campaign (select-keys car campaign-ks)]
    (assert (subset? campaign-ks ks)
            (str "Campaign keys missing from the car " (:id car) ": "
                 (difference campaign-ks ks)))
    (-> (apply dissoc car campaign-ks)
        (assoc :campaign campaign))))
 
(defn group-rows-by-car [cars-raw]
  {:pre [(sequential? cars-raw) (every? map? cars-raw)]
   :post [(sequential? %) (every? vector? %)]}
  (vals (group-by :id cars-raw)))
 
(defn join-campaigns [[car+campaign :as all]]
  {:pre [(sequential? all) (:campaign car+campaign)]
   :post [(:campaigns %)]}
  (-> car+campaign
      (assoc :campaigns
        (map :campaign all))
      (dissoc :campaign)))
 
(defn refine-car [car]
  {:pre [(car? car)]
   :post [(:discount %)]} ; keywords and :category_ref are optional
  (->> car
      (jdbc-array-to-set :category_ref)
      (jdbc-array-to-set :keywords)
      (#(update-in % [:keywords] (partial remove nil?))) ;; {NULL} => []
      (select-campaign)
      (compute-discount)))
 
(defn refine-cars
  "Process get-cars query result set - derive additional data, transform values into better ones
   There is one row per car and campaign, a car may have more campaigns - we pick the best one.
  "
  [cars-raw]
  (->> cars-raw
       (#(json->data % [:json]))
       (map nest-campaign)
       (group-rows-by-car)
       (map join-campaigns)
       (map refine-car)
   ))
 
(refine-cars [
              {:id 1
               :json "{\"discount_amount\":9000,\"discount_percent\":0}"
               :campaign_discount_amount 2000
               :campaign_discount_percent nil
               :use_campaign false
               :active true
               :keywords (db-array ["fast"])
               :category_ref (db-array [])}])

Downsides

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

1
Assert failed: (let [a (key m)] (or (nil? a) (instance? java.sql.Array a))) cars.clj:18 user/jdbc-array-to-set

лучше, чем не быстро провалиться, но не говорит о том, что было недопустимым значением и какое из тысяч автомобилей имело недопустимое значение.

Кроме того, проверки выполняются во время выполнения, поэтому они снижают производительность. Это может не быть проблемой с проверками типа (map?), Но может быть с f.ex. (Каждый?).

Как насчет дублирования?

Вы повторяете одни и те же проверки снова и снова? Затем вы можете либо скопировать их с помощью with-meta (они все равно попадут в метаданные), либо повторно использовать явно:

1
2
3
4
(defn with-valid-car [f] (fn [car] {:pre [:make :model :year]} (f car)))
  
(def count-price (with-valid-car (fn [car] (do-something car))))
;; or make & use a macro to make it nicer

Как насчет статических типов

Это выглядит как хороший пример для статических типов. И да, я с Явы и мне их не хватает. С другой стороны, несмотря на то, что статическая типизация решит основную категорию проблем, она создает новые и имеет свои недостатки.

А) На самом деле у меня довольно много «типов», поэтому для полного моделирования потребуется много классов:

  1. Необработанные данные из БД — машина с полями кампании и ключевыми словами, category_ref как java.sql.Array
  2. Автомобиль с ключевыми словами как последовательность
  3. Автомобиль с category_ref как последовательность
  4. Автомобиль с вложенным: кампания «объект»
  5. Автомобиль с вложенным объектом: best-campaign и с: rate (у вас может быть: rate там с начала, изначально установлен на nil, но тогда вам все равно нужно будет убедиться, что конечная функция установит для него значение)

Б) Основным преимуществом Clojure является использование общих структур данных — карт, векторов, ленивых последовательностей — и мощных, легко комбинируемых общих функций, работающих на них. Это делает интеграцию библиотек очень простой, поскольку все это просто карта (а не пользовательский тип, который необходимо преобразовать), и вы всегда можете преобразовать их с помощью своих старых функций хороших друзей — будь то определение запроса Korma SQL, набор результатов или HTTP-запрос. Статические типы убирают это.

C) Типы разрешают только подмножество проверок, которые могут вам понадобиться (то есть, если вы не используете Haskell!) — они могут проверить, что вещь является автомобилем, но не то, что возвращаемое значение находится в диапазоне 7… 42.

D) Некоторые функции не заботятся о типе, только его малая часть — например, jdbc-array-to-set заботится только о том, чтобы аргумент был картой, имел ключ и, если он установлен, значение было java.sql.Array .

Что еще там?

Вывод

Используя меньшие функции и условия до + post, я могу обнаруживать ошибки намного раньше, а также лучше документировать ожидаемую форму данных, особенно в случае деструктуризации в сигнатурах fn. Существует некоторое дублирование в условиях до / после, и сообщения об ошибках мало полезны, но намного лучше. Я предполагаю, что более сложные случаи могут препятствовать использованию core.contracts или даже core.typed / schema.

Какие стратегии вы используете? Что бы вы улучшили? Другие комментарии?

Я призываю вас раскошелиться, улучшить суть и поделиться своим мнением.

Обновления

  1. Лоуренс Крубнер рекомендует использовать dire для захвата аргументов и возвращаемого значения для предоставления полезного сообщения об ошибке
  2. Альф Кристиан рекомендует добавить больше тестов и интеграционных тестов, и, если этого недостаточно, использовать core.typed вместо: pre и: post ( пример )

Ссылка: Clojure: Как предотвратить «ожидаемую карту, полученный вектор» и подобные ошибки от нашего партнера JCG Якуба Холи в блоге Holy Java .
34
Share: