Hi Joe,

>  This is what I came up with to aggregate the data. It actually runs
> reasonably well. I'm sharing because I always enjoy reading other people's
> picoLisp code so I figure others may as well.

Yes, thanks!

I can't delve into the code's logic at the moment, so just let me make
some isolated comments:


> My approach is to load it in as follows:
> 
> (class +Invoice)
> (rel CustNum (+String))
> (rel ProdNum (+String))
> (rel Amount (+Number))
> (rel Quantity (+Number))

This is correct, but for a practical application it is not enough. All
four relations don't have an index, so the '+Invoice' cannot be found,
and they will be destroyed by the DB garbage collector (in case that
(dbgc) is called).

Also, if you have proper indexes, you might well be able to do the
operations below more efficiently with database operations, with the
additional benefit of persistence.


> (de Load ()
>   (zero N)

'N' seems not to be used.

>   (setq L (make (
>   (in "invoices.txt"

The open paren after 'make' is wrong. It causes the return value of the
'in' expression to be executed as a function. This may cause all kinds
of unpredictable things to happen.

Also, 'L' is a free variable then, possibly destroying an 'L' in the
calling environment. Better use something like

   (let L
      (make
         (in "invoices.txt"
            ...


>     (until (eof)
>       (setq Line (line) )
>       (setq D (mapcar pack (split Line "^I")))

Same here. Better do

      (until (eof)
         (let (Line (line)  D (mapcar pack (split Line "^I")))
            ...

or, even better because 'Line' is not needed

      (until (eof)
         (let D (mapcar pack (split (line) "^I"))
            ...


>       (link (new
>         '(+Invoice)
>         'CustNum (car (nth D 1))
>         'ProdNum (car (nth D 2))
>         'Amount (format (car (nth D 3)))
>         'Quantity (format (car (nth D 4))) )) ) ) ) ) ) T )

Instead of (car (nth D 1)) etc. you can use (get D 1) or simply 'car'.



> (de sortedGroup (List Fld)
>   (make
>     (let (Last NIL LastSym NIL)
>      (for This List
>       (let Key (get This Fld)
>         (if (<> Last Key)
>             (prog
>             (if LastSym (link LastSym))
>             (off LastSym)
>             (push 'LastSym Key)) )

As a minor improvement: An 'if' without and 'else' can always be
replaced with 'when', and then also the 'prog' is not needed

      (when (<> Last Key)
         (and LastSym (link @))
         (off LastSym)
         (push 'LastSym Key) )

The 'off' followed by 'push' is a bit redundant. You could use

   (setq LastSym (cons Key))

instead.

Just a few cents ...

Cheers,
- Alex
-- 
UNSUBSCRIBE: mailto:picolisp@software-lab.de?subject=Unsubscribe

Reply via email to