[Sooc-devel] SMap Comments

[Daniel R. <sta...@gm...>]

Some thoughts on SMap.
First: Maps seem pretty cool in general.  I once used ObjC's NSDictionary
class, which is a lot like a map, in that it associates keys of some type
with data of some type.

Second: It's interesting that the node class is defined and implemented in
the same files as the map class.  I actually didn't realize you could do
that in sooc, but due to C's procedural nature, I guess that makes sense
(and of course it's legal in other OO languages).  I suppose it was done
this way because SBSTreeNode is integral to the functionality of SMap.  It
deserves pointing out, though, that SBinaryNode, which is integral to the
functionality of SLinkedList, has its own file.  So this leads me to two
questions: (1) Would SBSTreeNode ever be useful to classes other than SMap,
meaning things might be more clear if it had its own file? (2) Could
SBSTreeNode have benefited from being a subclass of SBinaryNode (or vice
versa) since they do seem to share some ideas conceptually?

Regarding the latter question, I see that the biggest difference regarding
the way binary nodes and binary search tree nodes are linked in sooc is that
SBinaryNode uses a public data member whereas SBSTreeNode uses a method that
retrieves a private data member.  Presumably the public data member is
faster since it doesn't require whatever overhead occurs in calling a sooc
method.  On the other hand, this might be an inconsistency, but I'm not
sure.  I assume that because LinkedList is such a basic data structure, it
needed to be optimized for greatest speed/efficiency.  SMap is much more
esoteric data structure.

Your thoughts on the differences between SBinaryNode and SBSTreeNode and
whether they should be related would be appreciated!  In addition, as
mentioned before, your thoughts on whether SBSTreeNode should be its own
file would also be appreciated.

Third: I see that data2 (I think) is what holds the actual data that is tied
to the given key.  What does the plain "data" member do?  I was having some
trouble following the code that uses it.  On a related note, could you
possibly explain in "laymen's terms" how your binary search tree and map
implementation works?  I have a general idea from the code, but it would be
helpful to get the full picture, so to speak.

Fourth: Related to the question above, I am unclear as to the purpose of the
"up" member of SBSTreeNode.  From what I can tell, only "left" and "right"
are used to build a map (which makes sense).  I'm guessing "up" is intended
for branching purposes.  The name SBSTreeNode implies that these nodes can
be used to build a tree with multiple nodes attached to one root, rather
than simply being a very fancy linear linked list.  SMap clearly is not
intended to have this functionality, so it doesn't make use of the "up"
link.  If my understanding is correct, might this be another reason for
SBSTreeNode to be its own file, since it might be used by more complex
collections later down the line?

Fifth: How come set_up does not feature all the releasing and retaining that
takes place in set_left and set_right?  Is this an oversight or intentional?
 If intentional, could you explain the rationale?

Sixth: In s-map.h, the long description in the documentation for s_map_new
refers to a parameter called key_klass; however, its actual name is key_rtti
(I noticed that somewhere down the line you started using "rtti" in place of
"klass."  Some older files retain the previous convention.)  While I'm
pretty sure this is a typo, I chose not to modify it for now.  The
documentation for SMap is incomplete anyhow (I can probably write it at some
point, if you'd like to continue focusing on code; as it stands, I think I
already wrote a lot of the Collection documentation).

Okay, I think that's all I have to say; sorry for saying so much.  Nice
work, as always!  I hope my comments will prove useful.  Let me know what
you think, and I hope to hear from you soon!