Hi all,
I'm really making progress and having fun learning Lisp. One of the
comments said to get better I should post what I have so far, and ask
for improvements. Here goes:
(defmethod query-transaction ((tr <transaction>) category query)
(if (equalp category (category tr))
(setf query
(push (cons (description tr) (amount tr)) query)))
query)
This method takes a category and a query, which is a list of current
query results. If the category of this transaction matches, we build a
cons cell of the description and amount of this transaction, push it
onto the query list, and return it.
(defun get-transaction-query (category)
"Get list of transactions summaries for a certain transaction
category."
(let (query)
(dolist (tr (transactions *tm*))
(setf query (query-transaction tr category query)))
(reverse query)))
Allocate 'query' and loop over the list of transactions from the
transaction manager. 'query-transaction' takes a transaction and a
category. If they match, it returns a list containing the cons cell
results so far with the last one pushed on as well. I return the whole
list reversed for chronological order.
Is there a better way? (Of course. :-) Would it make sense to use
one of the mapping functions to populate the query list?
It should return a nested list like this:
(("description" . amount) ("description" . amount))
From: Joe Marshall
Subject: Re: Use (mapxxx) here rather than (dolist) ??
Date:
Message-ID: <u0i5qxrt.fsf@ccs.neu.edu>
"jonathon" <···········@bigfoot.com> writes:
> Hi all,
>
> I'm really making progress and having fun learning Lisp. One of the
> comments said to get better I should post what I have so far, and ask
> for improvements. Here goes:
>
> (defmethod query-transaction ((tr <transaction>) category query)
> (if (equalp category (category tr))
> (setf query
> (push (cons (description tr) (amount tr)) query)))
> query)
This is odd. This function is doing things that its caller ought to
be doing.
> This method takes a category and a query, which is a list of current
> query results. If the category of this transaction matches, we build a
> cons cell of the description and amount of this transaction, push it
> onto the query list, and return it.
>
> (defun get-transaction-query (category)
> "Get list of transactions summaries for a certain transaction
> category."
> (let (query)
> (dolist (tr (transactions *tm*))
> (setf query (query-transaction tr category query)))
> (reverse query)))
I'd write it like this:
;; Return the interesting 3 components of the transaction.
(defmethod query-transaction ((tr <transaction>))
(values (category tr)
(description tr)
(amount tr)))
(defun get-transaction-query (category)
(let ((result '()))
(dolist (tr (transactions *tm*))
(multiple-value-bind (cat desc amount)
(query-transaction tr)
(when (equalp category cat)
(push (cons desc amount) result))))
(nreverse result)))
Joe Marshall wrote:
> > (defmethod query-transaction ((tr <transaction>) category query)
> > (if (equalp category (category tr))
> > (setf query
> > (push (cons (description tr) (amount tr)) query)))
> > query)
>
> This is odd. This function is doing things that its caller ought to
> be doing.
Yes, that's true. I couldn't get stuff to work the way I wanted so I
just passed in the list to append to. Bad form.
> I'd write it like this:
Ah, much better, thanks. Now I get to work with values and
multiple-value-bind. Mapping will have to wait, since it really
doesn't fit in this application.
Thanks again.