Re: Solid modeling in Guile

[Alex Kost <alezost@gmail.com>]

Matthew Keeter (2016-08-19 00:44 +0300) wrote:

> Hi Guile-folks,
>
> I wrote a computer-aided design (CAD) tool that you may find interesting.
>
> It’s a solid modeling tool that uses Guile scripts to define objects (and
> constructive solid geometry + functional representations under the hood).
>
> Project page:  http://www.mattkeeter.com/projects/ao/
> Source:  https://github.com/mkeeter/ao

Wow, this looks very impressive, and you have so many great projects!

> I’d love feedback – Scheme is relatively new to me, so I’m sure there are
> more elegant ways to accomplish a lot of what the code implements.

You use ‘(- (inf))’ several times, it can be replaced with ‘-inf.0’.

From "bind/guile/ao/bind.scm":

--8<---------------cut here---------------start------------->8---
(define-module (ao bind))

(use-modules (srfi srfi-1))
(use-modules (ice-9 i18n))
(use-modules (system foreign))
--8<---------------cut here---------------end--------------->8---

Usually modules are defined like this:

--8<---------------cut here---------------start------------->8---
(define-module (ao bind)
  #:use-modules (srfi srfi-1)
  #:use-modules (ice-9 i18n)
  #:use-modules (system foreign))
--8<---------------cut here---------------end--------------->8---

From "bind/guile/ao/bounds.scm":

--8<---------------cut here---------------start------------->8---
(define-public (bounds-intersection bs)
    "bounds-intersection bs
    bs should be a list of '((xmin ymin zmin) (xmax ymax zmax)) lists
    Finds the intersection along each dimension
    If bounds are disjoint, returns empty interval (0, 0)"
    (let* ((lower (map car bs))
           (upper (map cadr bs))
           (xmin (apply max (map (lambda (b) (car b)) lower)))
           (ymin (apply max (map (lambda (b) (cadr b)) lower)))
           (zmin (apply max (map (lambda (b) (caddr b)) lower)))
           (xmax (apply min (map (lambda (b) (car b)) upper)))
           (ymax (apply min (map (lambda (b) (cadr b)) upper)))
           (zmax (apply min (map (lambda (b) (caddr b)) upper))))
    ;; Clamp intervals to empty (0,0) if bounds are disjoint
    (if (< xmax xmin)   (begin (set! xmin 0) (set! xmax 0)))
    (if (< ymax ymin)   (begin (set! ymin 0) (set! ymax 0)))
    (if (< zmax zmin)   (begin (set! zmin 0) (set! zmax 0)))
    (list (list xmin ymin zmin) (list xmax ymax zmax))))
--8<---------------cut here---------------end--------------->8---

All these cadr, caddr are hard to understand.  I would rather make some
level of abstraction for coordinates and bounds.  (if I understand it
right) the simplest would be:

--8<---------------cut here---------------start------------->8---
(define make-coordinates list)
(define x first)
(define y second)
(define z third)

(define make-bound cons)  ; bound of lower and upper coordinates
(define lower-coordinates car)
(define upper-coordinates cdr)

(define (extremum min-or-max coordinate coordinates)
  "Return minimal or maximal coordinate from COORDINATES.
MIN-OR-MAX is 'min' or 'max' procedure.
COORDINATE is either 'x', 'y' or 'z' procedure."
  (apply min-or-max (map coordinate coordinates)))
--8<---------------cut here---------------end--------------->8---

And now this 'bounds-intersection' can be written like this:

--8<---------------cut here---------------start------------->8---
(define-public (bounds-intersection bounds)
  "Finds the intersection along each dimension of BOUNDS.
If bounds are disjoint, returns empty interval (0, 0)."
  (let* ((lower (map lower-coordinates bounds))
         (upper (map upper-coordinates bounds))
         (xmin (extremum max x lower))
         (ymin (extremum max y lower))
         (zmin (extremum max z lower))
         (xmax (extremum min x upper))
         (ymax (extremum min y upper))
         (zmax (extremum min z upper)))
    ;; Clamp intervals to empty (0,0) if bounds are disjoint
    (when (< xmax xmin) (set! xmin 0) (set! xmax 0))
    (when (< ymax ymin) (set! ymin 0) (set! ymax 0))
    (when (< zmax zmin) (set! zmin 0) (set! zmax 0))
    (make-bound (make-coordinates xmin ymin zmin)
                (make-coordinates xmax ymax zmax))))
--8<---------------cut here---------------end--------------->8---

Actually using 'set!' for local variables is not considered a good
style, but, well, who cares :-)

Note that those 'first', 'second', 'third' procedures come from (srfi
srfi-1) module.

I'm not an expert, so do not consider this is the best (or even a good)
way.  I just wanted to point that instead of using lists, cars and cdrs
directly, it is better to have some higher-level procedures.

BTW, the indentation in .scm files is *very* unusual :-)

-- 
Alex