On Mar 3, 2009, at 4:53 AM, Chouser wrote:
>
>> (defn roman? [roman-string]
>> (and (not (empty? roman-string))
>> (re-matches
>> #"(?:M{0,3})(?:D?C{0,3}|C[DM])(?:L?X{0,3}|X[LC])(?:V?I{0,3}|I
>> [VX])$"
>> roman-string)))
>
> The normal idiom in Clojure is (seq x) instead of (not (empty? x)),
> and that works fine for Strings. Also, since you don't really need
> the specific return value of the test expressions, 'when' might be a
> better choice than 'and'.
>
Thanks for your comments Chris and Michel.
I'll keep the "seq" idiom in mind.
On the other hand, there is a convention in Common Lisp that I think
is worth observing in Clojure as well. It relates to the distinction
between "if" and "when". My view is that "if" is an expression, and
expressions return values. This is true whether the test is true or
not, so I am not a fan of implicit "else" expressions. Namely, I
believe that "if" should always contain 3 subexpressions. Moreover,
the "then" and "else" subexpressions by default consist of single
expressions. Of course they could be wrapped in "do" forms, but they
aren't automatically.
Conversely, "when" says "side effect". This is clear for 2 reasons.
First, there is no "else" clause, so "when" doesn't really work as an
expression. Furthermore, the forms in the "when" body are evaluated
in an implicit "do". Since only the final value is returned, any of
the earlier expressions in the "when" body are useful only for their
side effects.
For this reason I would argue against using "when" above. The
"roman?" predicate simply states that a string that isn't empty AND
that matches this pattern is a Roman numeral.
>> (defn roman-to-arabic-aux [roman-string]
>> (cond (empty? roman-string) 0
>> (empty? (rest roman-string)) (value (first roman-string))
>> (< (value (first roman-string)) (value (second roman-
>> string)))
>> (- (roman-to-arabic-aux (rest roman-string))
>> (value (first roman-string)))
>> :else
>> (+ (value (first roman-string))
>> (roman-to-arabic-aux (rest roman-string)))))
>
> To reduce the clutter here, you might consider using destructuring on
> roman-string to get locals for the first and rest parts:
>
> user=> (let [[a & b] "fie"] [a b])
> [\f (\i \e)]
>
You are probably right here. I am still thinking in terms of CL's
DESTRUCTURING-BIND, which yields an error if the pattern doesn't match:
(destructuring-bind ((a b) &rest tail) '() (list a b tail))
This is a problem since I don't know whether or not roman-string is
empty going into the "cond".
[D'oh! I just figured out how to do it the right way:
(destructuring-bind (&optional a b &rest tail) '( ) (list a b tail))
=> (NIL NIL NIL)]
However, Clojure just assigns nil to extra variables, so this works:
(defn roman-to-arabic-aux [roman-string]
(let [[a b & tail] roman-string]
(cond (empty? roman-string) 0
(nil? b) (value a)
(< (value a) (value b))
(- (roman-to-arabic-aux (cons b tail))
(value a))
:else
(+ (value a)
(roman-to-arabic-aux (cons b tail)))) ))
But this introduces its own awkwardness as I have to rebuild the tail
of roman-string (cons b tail) for each recursive call.
Aha! I'm destructuring too much. I could just do this:
(defn roman-to-arabic-aux [roman-string]
(let [[a b] roman-string]
(cond (empty? roman-string) 0
(nil? b) (value a)
(< (value a) (value b))
(- (roman-to-arabic-aux (rest roman-string))
(value a))
:else
(+ (value a)
(roman-to-arabic-aux (rest roman-string)))) ))
>> (def arabic-values '((1000 "M") (900 "CM") (500 "D") (400 "CD")
>> (100 "C") (90 "XC") (50 "L") (40 "XL")
>> (10 "X") (9 "IX") (5 "V") (4 "IV") (1 "I")))
>
> You might consider using a vector of vectors here. There's nothing
> particularly wrong with what you've got, but I've been noticing
> recently that almost every list you expressed directly in idiomatic
> Clojure code is a function or macro call. I think this is part of why
> I've found Clojure code to be less difficult to read than CL.
>
> It doesn't matter much in this case as the quote is clearly visible
> and the literal numbers aren't likely to be confused with function
> calls, but I thought I'd mention it anyway.
>
Good point. I'm still getting used to recursively CDR'ing across
vectors. I don't have any problems with the readability of the nested
list structure, but I understand that many Clojure programmers view a
reduction in parentheses as helpful.
>> (defn arabic-to-roman
>> ([n] (if (<= 1 n 3999)
>> (apply str (arabic-to-roman n arabic-values))
>> (format "%d cannot be converted." n)))
>> ([n num-list] (cond (empty? num-list) '()
>> (zero? n) '()
>> :else (let [[[arabic roman] & tail] num-list]
>> (if (>= n arabic)
>> (cons roman (arabic-to-roman (- n
>> arabic)
>> num-
>> list))
>> (arabic-to-roman n tail)))) ))
>
> You might consider using 'reduce' instead of recursion here.
> Alternatively, it's interesting to note that because of the ease with
> which you're destructuring arabic-values, it would be no more
> difficult if you had a single list (or vector) of alternating numbers
> and strings rather than nesting them.
>
I don't think "reduce" would actually work here. As you see, the test
(>= n arabic) determines whether we continue working with the
original num-list or proceed to the tail. I'm not sure of how to get
that behavior with "reduce".
Aloha,
David Sletten
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Clojure" group.
To post to this group, send email to clojure@googlegroups.com
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
-~----------~----~----~----~------~----~------~--~---
