CODEWITHSUNDEEP
clojure
Jyd
Jyd

Reputation: 227

Making comparison in Clojure

Developed a function in Clojure to check if inputs for the :maxlength and :minlength is 1-3 characters long and the text in associated column is within the range of min and max. Is there a cleaner way to do this? I'd appreciate if a more efficient method is suggested especially the if form.

(defn range-text-length
 "input = {:col val :minlength minlength :maxlength maxlength}
  Expects 3 parameter key/value combinations:
   :col The column containing the text value
   :maxlength integer value greater than zero up to 3 characters long
 Catch any rows where the column contains text that is shorter than the Minimum Length or longer than the Maximum Length"
 [row input]
 (let [{:keys [col  minlength maxlength ]} input
    minlength-count (count (str minlength))
    maxlength-count (count (str maxlength))
    max (read-string (str maxlength))
    min (read-string (str minlength))]
  (if (and (and (> maxlength-count 0) (<= maxlength-count 3) (number? max) (> max 0))
      (and (> minlength-count 0)(<= minlength-count 3)(number? min)(> min 0)))
  (and (>= (count (get row col)) min) (<= (count (get row col)) max))
  (throw (Exception. "length must be a positive integer value with no more than 3 digits.")))))

And I call the function thus:

(catch-out-of-range-text-length ["weert" "sertt" "qwertyuiopasdfg" "asert"] {:col 2 :minlength 2 :maxlength 15})

Upvotes: 0

Views: 176

Answers (4)

Frank C.
Frank C.

Reputation: 8096

I see you've already marked @noisesmith as answer. Whilst that was going on I was toying around and produced the following:

(defn range-text-length
  [row {:keys [col minlength maxlength] :as input}]
  (if-let [x (and (and (pos? minlength) (< minlength 1000))
                  (and (pos? maxlength) (< maxlength 1000))
                  (and (>= (count (get row col)) minlength)
                       (<= (count (get row col)) maxlength)))]
    true
    (throw (Exception. "length must be a positive integer value with no more than 3 digits."))))

Despite the variadic capability of and I separated for readability.

EDIT: Did away with the if-let:

(defn range-text-length
  [row {:keys [col minlength maxlength] :as input}]
  (or (and (and (pos? minlength) (< minlength 1000))
           (and (pos? maxlength) (< maxlength 1000))
           (and (>= (count (get row col)) minlength)
                (<= (count (get row col)) maxlength)))
    (throw (Exception. "length must be a positive integer value with no more than 3 digits."))))

Upvotes: 0

Thumbnail
Thumbnail

Reputation: 13483

I think the following is equivalent to yours: 

(defn range-text-length [row input]
  (let [in-range? (fn [n] (< 0 n 1000))
        {:keys [col  minlength maxlength ]} input
        ]
    (if (every? in-range? [minlength maxlength])
      (<= minlength (count (row col)) maxlength)
      (throw (Exception. "length must be a positive integer with no more than 3 digits.")))))

Upvotes: 2

Jaime Agudo
Jaime Agudo

Reputation: 8296

if clause tidied up and preconditions used to avoid polluting the real purpose of the function with the assumed preconditions. Some extra examples

(defn range-text-length
  "input = {:col val :minlength minlength :maxlength maxlength}
  Expects 3 parameter key/value combinations:
  :col The column containing the text value
  :maxlength integer value greater than zero up to 3 characters long
  Catch any rows where the column contains text that is shorter than the Minimum Length or longer than the Maximum Length"
  [row input]
  {: pre [(let [{:keys [col  minlength maxlength ]} input
                     minlength-count (count (str minlength))
                     maxlength-count (count (str maxlength))
                     max (read-string (str maxlength))
                     min (read-string (str minlength))
                     crc (count (get row col))]
                 (not (and (pos? maxlength-count)
                      (pos? minlength-count)
                      (pos? max)
                      (pos? min)
                      (<= maxlength-count 3) 
                      (<= minlength-count 3)
                      (>= crc min) 
                      (<= crc max))))]}
  (do (println "do my stuff with satisfied preconditions")))

Upvotes: -1

noisesmith
noisesmith

Reputation: 20194

First off: nested and is redundant. Since the code can't differentiate which part of the condition was false, there is no effect of nested calls to and as opposed to putting all the clauses in the same and.

Next, <, <=, >, and >= all take a variable number of arguments, and with a bit of refactoring you can combine calls that share a term.

Finally, rather than testing if something is greater than 0, you can directly test if it is positive with pos?.

(defn range-text-length
 "input = {:col val :minlength minlength :maxlength maxlength}
  Expects 3 parameter key/value combinations:
   :col The column containing the text value
   :maxlength integer value greater than zero up to 3 characters long
 Catch any rows where the column contains text that is shorter than the Minimum Length or longer than the Maximum Length"
 [row input]
 (let [{:keys [col  minlength maxlength ]} input
       minlength-count (count (str minlength))
       maxlength-count (count (str maxlength))
       max (read-string (str maxlength))
       min (read-string (str minlength))]
   (if (and (> 4 maxlength-count 0)
            (number? max)
            (pos? max)
            (> 4 minlength-count 0)
            (number? min)
            (pos? min))
     (>= max (count (get row col)) min)
     (throw (Exception. "length must be a positive integer value with no more than 3 digits.")))))

Upvotes: 0

Related Questions