HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

First Common Lisp vector math code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lispmathfirstcodecommonvector

Problem

I'm studying Common Lisp on my own. Coming from C++, Common Lisp feels kind of strange sometimes. I'd like to get some feedback on what I understood so far.

For learning purposes, I wrote this simple vector class package. The only requirements apart from correctness are non-destructiveness and performance. I want it to be as fast as possible. Is this the way to do it? Are there any serious flaws?

```
(in-package :cl-user)
(defpackage math
(:use :cl)
(:export :vector3
:create-vector3
:vector3-x
:vector3-y
:vector3-z
:neg
:mul
:div
:add
:sub
:dot
:len
:norm))
(in-package :math)

(defclass vector3 ()
((x :accessor vector3-x
:initarg :x
:initform 0
:type double-float)
(y :accessor vector3-y
:initarg :y
:initform 0
:type double-float)
(z :accessor vector3-z
:initarg :z
:initform 0
:type double-float)))

(defun create-vector3 (x y z)
(make-instance 'vector3 :x x :y y :z z))

(defmethod neg ((v vector3))
(create-vector3 (- (vector3-x v))
(- (vector3-y v))
(- (vector3-z v))))

(defmethod mul ((s double-float) (v vector3))
(create-vector3 (* s (vector3-x v))
(* s (vector3-y v))
(* s (vector3-z v))))

(defmethod div ((v vector3) (s double-float))
(create-vector3 (/ (vector3-x v) s)
(/ (vector3-y v) s)
(/ (vector3-z v) s)))

(defmethod add ((a vector3) (b vector3))
(create-vector3 (+ (vector3-x a) (vector3-x b))
(+ (vector3-y a) (vector3-y b))
(+ (vector3-z a) (vector3-z b))))

(defmethod sub ((a vector3) (b vector3))
(create-vector3 (- (vector3-x a) (vector3-x b))
(- (vector3-y a) (vector3-y b))
(- (vector3-z a) (vector3-z b))))

(defmethod dot ((a vector3) (b vector3))
(+ (* (vector3-x a) (vector3-x b))
(* (vector3-y a) (vector3-y b))
(* (vector3-z a) (vector3-z b))))

(defmethod len ((v vector3))
(sqr

Solution

Here are a few minor suggestions:

  • Use uninterned symbols instead of keywords in defpackage (e.g., #:neg instead of :neg) to avoid polluting the keyword package.



  • Fix your indentation.



  • Add a defgeneric with docs for each defmethod (you can fold defmethod inside defgeneric)



  • Decide if your objects are read-only; if they are, replace :accessor with :reader, if they are not, add mutator functions (e.g., scale in-place)



  • Rename norm to normalize (norm in math refers to the length of the vector)



  • Use (plusp x) instead of (> x 0)



  • Use (/ x) instead of (/ 1 x)



  • In the else clause in norm can just return the argument or (create-vector3 0 0 0) for clarity

Context

StackExchange Code Review Q#13414, answer score: 2

Revisions (0)

No revisions yet.