I was trying to find out if a string contained a capitalized letter and if it did I wanted to know the position of the character and its ASCII code. I might have refactored a little to much.

Given an input such as:

(is-capitalized "Meow mEow kItus kaTus")

My function returns:

((18 . 84) (11 . 73) (6 . 69) (0 . 77))

where the car of each sublist is the position in the string and the CDR of the sublist is the character ASCII code.

Given an input such as:

(is-capitalized "omg my cat attacked my face.")

My function returns: NIL

So I guess it works for what I want but I feel like I reinvented so many things.

How can I improve this code?

(defun string-to-ascii (str)
    "Converts string of characters into a list of ascii character codes                                
       Returns list of ascii character codes"
           :for i
         :in (coerce str 'list)
         :collect (char-code i)
         :into numbers
         :finally (return numbers)))

(defun return-capital-positon (list)
  "Returns list of positions where capitalized characters appear"
       :for i
     :in list
     :collect (if (and (> i 65) (< i 90))
                  (position i list)

(defun capital-hash (list-1 list-2 &key (test 'eql))
  "Return hash table where key: list-1 :pair list-2"
  (assert (= (length list-1)
              (length list-2)))
  (let ((table (make-hash-table :test test :size (length list-1))))
    (map nil (lambda (k v) (setf (gethash k table) v))
         list-1 list-2)

(defun do-hash (str)
  "Returns a hash table of key: position in string :pair ascii code character"
    (string-to-ascii str))
   (string-to-ascii str)))

(defun hash-table-alist (table)
  "Returns an association list containing the keys and values of hash table TABLE."
  (let ((alist nil))
    (maphash (lambda (k v)
               (push (cons k v)
                      alist)) table) alist))

(defun is-capitalized (str)
  "Returns a list of sublists in the form:                                                         
   ((position in string . ascii code of character)...)                                             
   if string contains capitalized characters                                                       
   otherwise returns nil"
  (remove 'nil (hash-table-alist (do-hash str)) :key #'first))


Check out map:

(def string-to-ascii (str)
  (map 'list 'char-code str))

Note that < takes &rest so you can write (< 65 i 90) instead of (and (> i 65) (< i 90)) (you also want <= instead).

Furthermore, return-capital-positon is quadratic in length of the input string - it can be made linear instead.

do- is a usually used as a prefix for iteration macros. Also, you call string-to-ascii in do-hash twice; you can use let to remove one call.

Your line breaks in hash-table-alist are very confusing.

You can use delete instead of remove in is-capitalized because hash-table-alist returns a fresh list.


I think your code is overkill.

(defun is-capitalized (str)
  (loop for char across str for code = (char-code char) for pos upfrom 0
     when (<= 65 code 90)
     collect (cons pos code)))

This has the added benefit that the return value increases in pos (i.e., it returns the reverse of your function).

Alternatively, you can actually avoid constants:

(defun is-capitalized (str)
  (loop for char across str for pos upfrom 0
    when (char= char (char-upcase char))
    collect (cons pos (char-code char))))

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.