robert-strandh/clouseau-issues

clouseau-issues's Issues

Why two methods on CONS and NULL with the same body?

In objects/package.lisp there are two groups of two methods each, one group
specialized to PACKAGE-NICKNAME-PLACE and the other to
PACKAGE-LIST-PLACE-MIXIN and in each group, there are two methods,
one specialized to NULL and one to CONS. Why not have a single method
in each group, specialized to LIST?

Comment in ENSURE-CHILD may be wrong or misleading.

In the file place.lisp, the comment in ENSURE-CHILD states that there
will typically be a single container, so an alist is used which will
typically have a single element. However, inspecting an arbitrary
vector seems to suggest that the alist contains two element, the last
one with a key of NIL.

Inheritance of slot %CELL in places.

In the file place.lisp, the class BASIC-PLACE contains a slot %CELL
with a :READER CELL. In the class PSEUDO-PLACE-MIXIN, an additional
:READER named VALUE is added to this slot, and in the class
ROOT-PLACE a :READER named VALUE and a writer named (SETF VALUE) are
added to this slot.

I can't see any reason why some code would want to refer to this slot
either as (CELL ...) or (VALUE ...) when a ROOT-PLACE is manipulated.
Indeed if it did, it would be very confusing. So then, would it not
be better to have (say) a class PLACE that is the parent of both
BASIC-PLACE and ROOT-PLACE, and that has no slot %CELL, and to have
ROOT-PLACE be a subclass of this new common parent?

inspector-state is sometimes referred to as just state

At the end of the file protocol.lisp is specified the "inspector state
protocol". Since the term "state" is used also for "object state" it
seems to me important to distinguish the two. I therefore suggest
that the first parameter of the generic function
PRESENT-INSPECTED-OBJECT-GRAPH and its method be renamed form STATE to
INSPECTOR-STATE.

In the file pane.lisp, PRESENT-INSPECTED-OBJECT-GRAPH is called with a
first argument of (STATE PANE) suggesting that the state of the
INSPECTOR-PANE-MIXIN is in fact an inspector state. It would then
seem more reasonable to rename the slot and its accessors so that the
they mention INSPECTOR-STATE rather than just STATE.

Also in the file pane.lisp, there are methods on ROOT-OBJECT and (SETF
ROOT-OBJECT) with a parameter named INSPECTOR-STATE but the class of
this parameter is INSPECTOR-PANE-MIXIN. But INSPECTOR-PANE-MIXIN is
not a subclass of INSPECTOR-STATE, so this looks a bit strange.

INSPECTOR-STATE may not be a tree as claimed?

In the file state.lisp, the comment suggests that the state is a tree
of inspected places and objects, but the code suggests that it is not
only not a tree, but that it can contain cycles. The methods on
INITIALIZE-INSTANCE and SHARED-INITIALIZE specialized to
INSPECTOR-STATE set the ROOT-PLACE of an INSPECTOR-STATE (call it IS)
to and instance of the class ROOT-PLACE having IS as its :CONTAINER.

Multiple uses of the term "state".

There is a file named state.lisp that has comments indicating that it
defines an object type called STATE, but in fact, it defines an object
type named INSPECTOR-STATE. Since a STATE is really an instance of
the class INSPECTED-OBJECT, the comments are misleading.

In a previous issue, I recommended that INSPECTED-OBJECT be renamed to
STATE, but even if that turns out to be a good idea, there is this
confusion with INSPECTOR-STATE.

Why the attempt to make the object hierarchy look flatter than it is?

There seems to be a large amount of code with the purpose of making
the object hierarchy look flatter than it is. I already mentioned
methods that make it possible to call (F X) but that end up calling (F
(G X)), thereby making it look like X has an F, even though F has a G
and G has an X.

Similarly, when MAKE-INSTANCE is called to create an F, there is a
large amount of code that makes it possible to pass :X as an
alternative to :G (MAKE-INSTANCE 'X). This possibility then makes it
necessary to have yet more code to check that :X and :G are not both
given.

Perhaps there are some technical reasons for this organization, but to
a person trying to understand the system, it is terribly confusing,
for what seems to be some minor convenience.