Hi Ed,
I got time to look through this patch, in the end.
Code external to the library doesn't really need to know about how the
o_shortest_distance() function is implemented. I strongly recommend putting
all the declarations of the of the o_<type>_shortest_distance() functions
into libgeda/include/prototype_priv.h instead of prototype.h, so that only
libgeda code can use them directly. Don't forget that it's much harder to
remove a function from a library's public interface than to add one!
Also, a few other points.
Firstly, is it possible to make your editor highlight trailing whitespace?
In my Emacs 22 .emacs, I have:
;; Make file buffers use tab-completion and highlight trailing whitespace
(add-hook 'find-file-hook
(function (lambda ()
(make-local-variable 'show-trailing-whitespace)
(setq show-trailing-whitespace t))))
Secondly, you're using g_assert() quite a lot. In particular, you assert
that the object isn't NULL in o_shortest_distance(), but you then do
exactly the same assertion in each of the type-specific functions!
A few months back, I thought carefully about this and wrote a detailed blog
post about my conclusions. In particular, we assume that libgeda works --
that is, libgeda functions always pass valid data to other libgeda
functions. We also try to avoid killing the application if we possibly can.
http://www.peter-b.co.uk/blog/2007/11/error-reporting-handling-in-libgeda.html
Instead of causing an assertion failure when the input isn't valid,
consider issuing a g_critical() warning and then returning your 'couldn't
be calculated' value. Have you considered using a negative return value to
signify an error, instead of the maximum double?
Finally, instead of using DBL_MAX, please use G_MAXDOUBLE.
Cheers,
Peter
_______________________________________________
geda-dev mailing list
[email protected]
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev
