Request input regarding code style / best practices

Question

I'm trying to implement a programming language in Common Lisp by following this implementation in Java (https://craftinginterpreters.com/control-flow.html).

One of the things that was being really bothersome is plenty of slot-values everywhere.

Granted I can add a line in each function with with-slots, but that's also repetitive, and given that my symbols are not :used but I refer to them by the package, as there is plenty of symbols from many modules and I don't want to lose track from where they come from, even with-slots requires the full qualified name. All this seems to me like very bad code smell.

I googled and found rutils. With its @object.slot accessor I managed to clean up my code immensely. Just look at the last commit https://github.com/AlbertoEAF/cl-lox/commit/84c0d62bf29bff9a2e0e77042a9108c168542b96?diff=split excerpt:

(I could paste the code but all of it is available on that repo, I wanted to show the diff highlights)

Not only it removes some characters, but more importantly, there's one less level of depth (slot-value call) and parenthesis to think about - which helps a lot but for the trivial functions.

It gets worse when I have lots and lots of symbol names and can no longer export them all because I start having conflicts in symbols.

Can you guys give me input on the code? This looks much better now but I'm curious as if to are better ways to go about it?

Thanks!

[codereview.se] is the appropriate place to ask coding style questions. — Barmar, Jun 04 '20 at 01:00
You should post code as text, not images. Images are hard to read (you have to expand them into another window), bad for screen readers, and impossible to cut and paste into answers. — Barmar, Jun 04 '20 at 01:01
Experienced CLOS programmers just get used to use `slot-value` and/or `with-slots`. — Barmar, Jun 04 '20 at 01:03
I practically never use slot-value but define a `:reader` function. In case the slot is mutable, it depends on who is expected to mutate it: the user? then I define an `:accessor` function. Internal code in the same package? a `:writer` function with an unexported name. Accessors/reader/writers are more readable and can have :around method etc that help enforce properties/behaviours, unlike slot-value. — coredump, Jun 04 '20 at 08:11
If the slot is part of the public interface of the class, there should be an accessor for it (which accessor may or may not have a writer, and may or not actually read a slot: that's no business of any user of the class). Anything using `slot-value` knows intimate details of the implementation of the class, and thus is part of the implementation of the class. — , Jun 04 '20 at 08:42

Gwang-Jin Kim · Accepted Answer · 2020-06-06T15:07:00.167

Defining an accessor function in the class definition would also save you some typing while not having to use non-standard syntax

If you define a CLOS class like this:

(defclass person ()
  ((name
    :initarg :name)))

(defparameter *p* (make-instance 'person 
                     :name "John"))

The only way to access then the slot value of name slot in *p* are:

(slot-value *p* 'name)
;; "John"

(with-slots (name) *p*
  name)
;; "John"

(with-slots ((nm name)) *p*
  nm)
;; "John"

But if you define for each slot an :accessor, you can use the function name given as argument of :accessor and don't have to use slot-value or with-slots, to access and to mutate it (setf-able!).

(defclass person ()
  ((name
    :initarg :name
    :accessor nm)))

(nm *p*)
;; "John"

(setf (nm *p*) "Doe")
;; "Doe"

(nm *p*)
;; "Doe"

The convention however is, to use the slot name also as the :accessor method's name:

(defclass person ()
  ((name
    :initarg :name
    :accessor name))) ;; but better use the slot name as accessor

(name *p*)
;; "John"

(setf (name *p*) "Doe")
(name *p*)
;; "Doe"

The :accessor method (generic function) is specific for the objects of this class. Therefore you don't need to have fear of a name-space clash.

(defclass house ()
  ((address
     :initarg :address
     :accessor addr))) 
;; you cannot use `address` because it is already occupied 
;; by a system's function/symbol -> see under `(describe 'address)`
(defparameter *h* (make-instance 'house :address "Bakerstreet 1"))

(name *h*) 
;; EVAL: undefined function name OR: 
;; NO-APPLICABLE-METHOD error (in the case
;; that other classes exist with a `name` accessor method.

(addr *h*)
;; "Bakerstreet 1"
(addr *p*)
;; *** - NO-APPLICABLE-METHOD: When calling #<STANDARD-GENERIC-FUNCTION ADDR>
;;       with arguments (#<PERSON #x1B06B79E>), no method is applicable.
;;

Request input regarding code style / best practices

1 Answers1

Defining an accessor function in the class definition would also save you some typing while not having to use non-standard syntax