Site icon Random Thoughts

Offensive Code

larsmagne23

While writing code to format Roman Numerals, I was made aware of the Rosetta Stone collection of code snippets to do the same.  I read the code, and after looking at it for a few minutes, I found myself oddly offended.

By the code.

Here it is:

(defun ar2ro (AN)
  "translate from arabic number AN to roman number,
   ar2ro(1666) returns (M D C L X V I)"
  (cond
   ((>= AN 1000) (cons 'M (ar2ro (- AN 1000))))
   ((>= AN 900) (cons 'C (cons 'M (ar2ro (- AN 900)))))
   ((>= AN 500) (cons 'D (ar2ro (- AN 500))))
   ((>= AN 400) (cons 'C (cons 'D (ar2ro (- AN 400)))))
   ((>= AN 100) (cons 'C (ar2ro (- AN 100))))
   ((>= AN 90) (cons 'X (cons 'C (ar2ro (- AN 90)))))
   ((>= AN 50) (cons 'L (ar2ro (- AN 50))))
   ((>= AN 40) (cons 'X (cons 'L (ar2ro (- AN 40)))))
   ((>= AN 10) (cons 'X (ar2ro (- AN 10))))
   ((>= AN 5) (cons 'V (ar2ro (- AN 5))))
   ((>= AN 4) (cons 'I (cons 'V (ar2ro (- AN 4)))))
   ((>= AN 1) (cons 'I (ar2ro (- AN 1))))
   ((= AN 0) nil)))

First of all, I really don’t like the repetition of the constants (1000 … 1000, 900 … 900).

Second of all, I don’t like that the subtractive elements (CM for 900) are in there explicitly, since they can be easily derived.  You may argue that it’s defensible in this case, since there aren’t really that many possible Roman “digits”, but writing repetitious code like that is error prone.  (And virtually all the Rosetta Stone examples, in all the languages, do the subtractive elements explicitly.)

Which leads me to the third point, which is that the algorithm is wrong.  9 is encoded as VIV, since whoever typed this forgot to write the “9” line.

The offensive part is, I think, that this is code that no humans should write.  It’s so pattern-based that one should write code that generates that code.  But in that case, you might as well just write the algorithm “properly”, which leads to smaller code, anyway:

(defvar roman-numeral-mapping
  '((1000 . "M") (500 . "D") (100 . "C") (50 . "L")
    (10 . "X") (5 . "V") (1 . "I")))

(defun format-roman-numeral (number)
  "Format NUMBER into a Roman numeral.
Roman numerals look like \"XCVII\"."
  (let ((mapping roman-numeral-mapping)
         values roman elem subtract)
    ;; Add the subtractive elements ("IV", etc) to the mapping.
    (while (setq elem (pop mapping))
      (push elem values)
      ;; We use the feature that the subtractive element is alternatively
      ;; one or two elements further along in the list.
      (when (setq subtract (elt mapping (mod (1+ (length mapping)) 2)))
        (push (cons (- (car elem) (car subtract))
                    (concat (cdr subtract) (cdr elem)))
              values)))
    ;; Compute the Roman numeral.
    (dolist (value (nreverse values))
      (while (>= number (car value))
        (push (cdr value) roman)
        (setq number (- number (car value)))))
    (apply 'concat (nreverse roman))))

Remove the comments, and it’s fewer lines.

(Now, this might be characterised as “smarty-pants programming”.  The normal way to have a variable that flips between 0/1 would be to, well, have a variable that flips between 0/1.  Instead I’m using the decreasing length of a list (modulo 2) to get the same effect.  It’s slower, and it makes the code more obscure, but, hey, it saves a couple of lines.  Other than that, I don’t think I’ve cheated much.)

I don’t often see code that I think is downright offensive.  I mostly see good code, or bad code, or WTF code, but seldom code that squicks me out.  But the Rosetta code did.

Exit mobile version