On Sun, Dec 7, 2008 at 6:50 PM, Robert Kawulak <robert.kawulak@gmail.com> wrote:
From: Stjepan Rajko
I think I get your idea. However, it is still a fresh mental model and I'm not confident about all the hidden problems that may be involved. Anyway, here are a few loose comments of mine.
The above treatment covers the current behavior of the library, allows dealing with issues such as the floating point case
Actually, I would see something different as a perfect solution to the FP problem. If an "exact floating point" type could be provided (out of scope of this library), being a wrapper for float/double and making sure that its underlying value is always truncated, you could perform comparisons (and all the other operations) that are repeatable, without the possibility that a comparison that once succeeded will later fail. Does it sound sensible?
Yes, and it would fit with what your library currently supports. It seems like a class like that would be uber-useful anyway (considering how much attention problems with floating point comparisons got in this review, I am wondering why I never considered this or saw this considered in the past).
And the "exact floating point" could be implemented as a monitored value that would truncate the value on assignment... Isn't it getting crazy? :D
I find that such craziess often accompanies well designed libraries ;-)
AFAICT, It requires no changes to the implementation.
Not exactly: - there are asserts checking for the invariant, which then should be removed,
Yes, you are right.
- given the modified set of concepts and assumptions, the current names in the code do not necessarily fit the purposes (e.g., constraint is not constraint anymore, it is the trigger of the monitor callback, which in turn is the former error policy).
I agree here as well. Your nomenclature fits your use case very well, whereas a more abstracted nomenclature would probably be more vague in all of the individual scenarios it supported.
I also think that it should be analysed whether the current design really fits monitored values in an optimal way. The implementation seems optimal for constrained values, which doesn't have to be the case for monitored values. And the other way round -- maybe a monitored value wouldn't be a best choice for implementation of constrained value?
I am inclined to think that the implementation you have now (minus the asserts and the nomenclature) covers all of the uses fairly well. Existence of in-between use cases (conditionally monitored values, e.g., log whenever the temperature is over 35 degrees Celsius) leads me to believe that neither of {monitored value, conditional value} should be implemented in terms of the other. I think they should be implemented under a common abstraction, and I think your implementation implements that abstraction (again, minus the asserts and the nomenclature).
I think the idea of monitored values as the generalisation of constrained values seems reasonable and elegant. Unfortunately I doubt I would have time and resources to transform Constrained Value library into Monitored Value library (this library already consumes more of my free time than I have :P). However, I think that if such library is created in the future, then Constrained Value library may be re-implemented in terms of it (as an extension).
The nomenclature problems, the need to revamp the documentation, and uncertainties about what lies hidden in the changes are probably reasons enough to justify stick with what you have, at least for now. I think your library will have a field day with c++0x - in addition to all other goodies you can take advantage of, you can solve the nomenclature problem elegantly with template typedefs (should you choose to extend the scope of the library).
One more thought -- monitored value may be actually a case of even more general idea, something similar to transparent proxy in .Net, or at least something that could be called a "universal wrapper". This wrapper would allow to define callbacks invoked every time the value is being set (as in monitored value) and get. (I think somebody might have already been discussing this idea with me, but it would be a long time ago and I can't remember.)
Looks like you have your work cut out for you for quite a while :-) For now, I would be plenty happy if you just took out the asserts, or made them optional (with defaulting to asserts, if you wish). That way I can at least start experimenting with your library in a monitored_value context, and let you know how it goes (I have use cases for this). Best, Stjepan