Статьи

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

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

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

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

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

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

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

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

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

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

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

(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))))))
(require '[cheshire.core :refer [parse-string]])
(require '[clojure.set :refer [subset? difference]])

(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)}])

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

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

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

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

(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 наверняка обнаружил бы и исправил коренные причины гораздо быстрее):

## 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
...

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

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

(require '[cheshire.core :refer [parse-string]])
(require '[clojure.set :refer [subset? difference ]])
(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

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

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

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

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

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

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

(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 ( пример )