traits class specializations in header files
One of Peter's recent commits (moving Traits::TreeIterator<SPObject *> into a separate header file) started me thinking more about what should go in a separate header file and what shouldn't.
In _general_, I think "one class, one header" is the right approach, but some auxiliary classes (for example, traits specializations) have a more intimate relationship than most classes, and don't introduce any dependencies of their own.
Any thoughts on where a good place to draw the line might be, in general?
-mental
On Sat, Aug 07, 2004 at 02:08:33AM -0400, MenTaLguY wrote:
One of Peter's recent commits (moving Traits::TreeIterator<SPObject *> into a separate header file) started me thinking more about what should go in a separate header file and what shouldn't.
In _general_, I think "one class, one header" is the right approach, but some auxiliary classes (for example, traits specializations) have a more intimate relationship than most classes, and don't introduce any dependencies of their own.
Any thoughts on where a good place to draw the line might be, in general?
Apart from the class-per-file guideline, I was motivated by reducing compile times: sp-object.h is #included by almost everything (being a base class of all shapes), whereas sp-object-tree-iterator.h is currently #included by exactly one file: sp-object.cpp.
Of course our primary concern should be developer time. Reducing compile times does improve developer efficiency, but that should be weighed against other time costs such as finding what header files need to be included to get the compile to work, or possibly the cost of wrongly assuming that it won't work when the compile fails.
Comments on intimacy: On one hand, giving facts about a type might well be seen as belonging with the definition of that type. On the other, traits classes might rather be seen as showing ways in which the type can be used.
If the programmer knows that to use a Foo as a BarTrait requires that one #include "Foo-BarTrait.h" (or other regular pattern used consistently throughout inkscape), then I think it's fine.
Looking at other instances of splitting things into a separate header file:
I'm confident with the decision to split sp-desktop-widget.h from desktop.h. E.g. because it leaves the latter with no gtk dependency.
A much more questionable case is splitting nr-matrix.h into many files. See the end of that file for comments on this.
pjrm.
On Sat, 2004-08-07 at 04:04, Peter Moulder wrote:
On Sat, Aug 07, 2004 at 02:08:33AM -0400, MenTaLguY wrote: Apart from the class-per-file guideline, I was motivated by reducing compile times: sp-object.h is #included by almost everything (being a base class of all shapes), whereas sp-object-tree-iterator.h is currently #included by exactly one file: sp-object.cpp.
Have you measured the compile-time impact of having Traits::TreeIterator<SPObject *> in sp-object.h?
Comments on intimacy: On one hand, giving facts about a type might well be seen as belonging with the definition of that type. On the other, traits classes might rather be seen as showing ways in which the type can be used.
They certainly do serve a documentation role as well.
If the programmer knows that to use a Foo as a BarTrait requires that one #include "Foo-BarTrait.h" (or other regular pattern used consistently throughout inkscape), then I think it's fine.
That may depend on how many traits classes are typically needed at once, also. The number is going to increase, since there are some other distinctions/abstractions that I think are important.
One in particular is equality... I'm seriously considering adding a Traits::Eq template which draws a distinction between "equal" and "equivalent" similar to Scheme's equal? versus eqv?; I imagine most of the generic algorithms will end up using it.
If every generic algorithm has a shopping list of traits headers required (some of which are necessarily implementation details rather than obvious parts of its interface), coming up with the required headers could get very obnoxious quickly (it's already a little obnoxious with nr-matrix headers).
I think it is pretty important that the generic algorithms "just work" (and are efficient, but that is an unrelated issue), otherwise people will simply not use them.
Looking at other instances of splitting things into a separate header file:
I'm confident with the decision to split sp-desktop-widget.h from desktop.h. E.g. because it leaves the latter with no gtk dependency.
Yes, that's one I've been wanting to do for a while, although I actually think sp-desktop-widget belongs in widgets/, if it is possible.
-mental
On Sun, Aug 08, 2004 at 02:31:16AM -0400, MenTaLguY wrote:
Have you measured the compile-time impact of having Traits::TreeIterator<SPObject *> in sp-object.h?
For developers, recompilation time is most significant. Separate headers means that changes to what might go in a separate header cause only sp-object.o to be rebuilt rather than most of inkscape.
They certainly do serve a documentation role as well.
For the most part better served by a comment.
However, an advantage of code over comment is that code is checked by the compiler.
That may depend on how many traits classes are typically needed at once, also. The number is going to increase, since there are some other distinctions/abstractions that I think are important.
If we decide to put each traits class in a separate header for the moment, then we can later add `#include the-separate-header' to whatever places we think appropriate wenn it becomes appropriate to do so.
One in particular is equality... I'm seriously considering adding a Traits::Eq template which draws a distinction between "equal" and "equivalent" similar to Scheme's equal? versus eqv?; I imagine most of the generic algorithms will end up using it.
The STL approach is to allow the user to specify a comparison predicate. We should use that approach in preference to different equality traits if we can.
pjrm.
On Sun, 2004-08-08 at 04:52, Peter Moulder wrote:
On Sun, Aug 08, 2004 at 02:31:16AM -0400, MenTaLguY wrote:
Have you measured the compile-time impact of having Traits::TreeIterator<SPObject *> in sp-object.h?
For developers, recompilation time is most significant. Separate headers means that changes to what might go in a separate header cause only sp-object.o to be rebuilt rather than most of inkscape.
A traits specialization is not going to change unless the primary class does (and seldom even then), and the addition of new specializations is a fairly rare occurrence.
They certainly do serve a documentation role as well.
For the most part better served by a comment.
Comments also cause recompilation. Do they belong in a separate header?
I guess that's actually a serious question. Recompilation times have discouraged me from going back and commenting sometimes when I felt lazy. I believe Doxygen permits documentation in separate headers or in the .cpp file.
On the other hand, that may mean fewer people read the comments.
However, an advantage of code over comment is that code is checked by the compiler.
Since templates are lazily evaluated, traits specializations are not well-checked by the compiler, beyond their existence or non-existence.
Actually, as a result, if the primary class is not updated without having its traits specializations updated it may not be apparent for a while. So that might be an argument for keeping them next to the primary class, where they will be visible.
("oh, hey, this traits specialization uses this method I just changed the signature of...")
I guess there's a similar issue with comments as well, though obviously the comments getting out of sync will not directly impact the correctness of the code.
That may depend on how many traits classes are typically needed at once, also. The number is going to increase, since there are some other distinctions/abstractions that I think are important.
If we decide to put each traits class in a separate header for the moment, then we can later add `#include the-separate-header' to whatever places we think appropriate wenn it becomes appropriate to do so.
That's true. I guess the only real issues are human rather than technical.
One in particular is equality... I'm seriously considering adding a Traits::Eq template which draws a distinction between "equal" and "equivalent" similar to Scheme's equal? versus eqv?; I imagine most of the generic algorithms will end up using it.
The STL approach is to allow the user to specify a comparison predicate. We should use that approach in preference to different equality traits if we can.
Well, the motivation is that in some cases more than one notion of equality is required in the same place (c.f. comments in longest_common_suffix).
But yes, the predicates ought to be configurable. Eq would still be desirable for specifying default predicates (STL also uses traits classes for its defaults).
-mental
On Sun, Aug 08, 2004 at 01:05:23PM -0400, MenTaLguY wrote:
On Sun, 2004-08-08 at 04:52, Peter Moulder wrote:
[Recompilation as one reason to put even traits classes in their own header file.]
A traits specialization is not going to change unless the primary class does (and seldom even then), and the addition of new specializations is a fairly rare occurrence.
Changes:
- Creation
- Changes to the base traits class: e.g. adding an is_null() method where previously traits users would compare against T.null.
- Cleanups, e.g. using convenience function.
- Changes to the SPObject (in this case) class. (Such changes don't count for deciding whether to use separate header file, as mental notes.)
- Addition or changes to documentation comments.
Mental's notes about doxygen allowing comments in different places are interesting. For functions (methods), putting the comment with the definition is good, especially if doxygen can do something useful for it. (Other reasons for putting documentation with the definition: Obviously one is so that it's more likely to be kept up to date. Another is that tags programs default to locating the definition rather than the declaration.)
For comments about a class, I guess the corresponding .cpp file is a good place -- though this would greatly benefit from having just one .cpp file per class, which in turn suggests one .h file per class.
Comments about instance data are probably best in the class definition itself, though a viable alternative is to accompany the documentation on the class as a whole.
pjrm.
On Sun, 8 Aug 2004, MenTaLguY wrote:
A traits specialization is not going to change unless the primary class does (and seldom even then), and the addition of new specializations is a fairly rare occurrence.
They certainly do serve a documentation role as well.
For the most part better served by a comment.
Comments also cause recompilation. Do they belong in a separate header?
I guess that's actually a serious question. Recompilation times have discouraged me from going back and commenting sometimes when I felt lazy. I believe Doxygen permits documentation in separate headers or in the .cpp file.
On the other hand, that may mean fewer people read the comments.
Documenting the .cpp file probably makes better sense than moving it to a separate file, if the name of the game is keeping the comments updated.
Really, though, the true solution is to find ways of better isolating interfaces between subsystems, forward declarations, and other such techniques for cutting down the include hierarchy. Pondering about comment location seems like a micro-optimization twiddle. ;-)
I think if AST and the extensions are done well, we may be able to make some serious inroads into the recompilation trouble.
Although I wonder if the simplest solution to the problem would be to take up a collection to get Mental a faster computer. ;-)
Bryce
participants (3)
-
Bryce Harrington -
MenTaLguY -
Peter Moulder