GNOME Bugzilla – Bug 94110
proposed API additions to Glib::TimeVal
Last modified: 2004-12-22 21:47:04 UTC
The attached patch contains the proposed API additions to Glib::TimeVal. I hope that they are useful. "subtract", "add_millisecond" and "add_microsecond" were already present, so I added "add", "subtract_millisecond", "subtract_microsecond", "=+" and "=-" as member operators with Glib:TimeVal and long as possible arguments. Additionally a conversion operator to double was added. I know that such operators are not considered "good" and "useful" by everyone. The other option would be to add a memberfunction that does the conversion. The non-member operators "+" and "-" are now also present for Glib::TimeVal and long (seconds) arguments. For the sake of completeness also "add_second" and "subtract_second" have been added as member functions. The patch includes also a little bit of documentation. I dare to say, that not much documentation is needed for this class. The second attached file contains a little quick and dirty test program for the additions. I include it in case you want to give it a try.
Created attachment 11237 [details] [review] The patch containing the proposed API additions
Created attachment 11238 [details] a test program for the API additions
Created attachment 11239 [details] [review] the same patch as above but this time including a Changelog entry
+#include <glib/gtimer.h> Do you really need that line? We try to avoid unnecessary includes of the c headers. Otherwise looks good for me.
I must admit that I did not have a look at the include statements. The line including gtimer.h is present in the unpatched file if I am not completely mistaken. I did not put it there. After having a look at gtimer.h I came to the same conclusion as you: IMHO unnecessary.
In current CVS the include is not there! Neither in anoncvs as your patch proves with the line '+#include <glib/gtimer.h>'. ^^^ Probably you have to update your local copy of the gtkmm module. Murray, shall I commit the changes (without the "include", of course)?
I like to give Daniel Elstner a chance to approve glibmm changes first.
Either I have messed up the cvs update (and I am sure I updated the sources before doing the diff) or (which is more likely) I copied the line without noticing it from the .cc file to the header file when I moved some functions. I will be more careful next time. Sorry
I don't think I like the operator double conversion. Maybe a get_total_seconds() would be better. Maybe that already exists.
Such a member function does not yet exist as I have only added the conversion operator to double. As you can see in my first posting, I was pretty much aware, that not everybody might like this approach. I would rather call the member function "to_double", but I don't have time right now to look through the name convention for conversion member functions in glibmm and gtkmm. Do you want an updated patch? If yes, is "get_total_seconds()" the preferred name?
get_total_seconds() seems better than to_double() because the double is expressed in the return type, but to_double() doesn't describe what the number actually is - hours/minutes/seconds? If get_total_seconds() makes sense to you then let's use it. I haven't looked that the patch properly. Yes, an updated patch is necessary. Thanks.
GTimeVal, which is wrapped by Glib::TimeVal, is the same struct as the struct timeval returned by the gettimeofday() UNIX call. It contains two long values, one for the seconds and one for the microseconds. Thus it has little to do with hours/minutes/seconds. The function in question is more a conversion from an integer representation of a certain time interval to a float representation of such an interval. And thats why I came up with "to_double". Scott Meyers proposes the name "asDouble" for a similar function in his book. "to_double" or "as_double" would in this case make more sense to me than "get_total_seconds()". I know that the double is expressed in the return type, but I still think that they are more suitable. Anyone is welcome to suggest a different name. My imagination is somewhat limited in this area and I can't come up with anything that is suitable and does not contain "double" right now.
Then you'll have to explain exactly why you do that calculation to get the double. Why would I want a double instead of an int?
Sometimes it is more convenient to work with the exact time values , seconds and microseconds, stored in seperate long values (example: timeout interval). Sometimes a double representation of this time value is more convenient, like in scientific applications. I have used a double representation of the current time acquired with the gettimeofday() UNIX system call several times to calculated the approximate mean of a measured value which was weighted with the length of the time interval in which it appeared. I believe, that the two representations (long+long and double) are both useful for different purposes, but I can of course remove the member function from the patch if you want that.
Created attachment 11419 [details] [review] new version of the patch, double conversion moved to a member function
explanation of the new patch: I now I do know why I did include this file. Sorry that I could not remember this detail before: #include <glib/gtimer.h> is needed, because it defines #define G_USEC_PER_SEC 1000000 I believe it to be more consistent to use G_USEC_PER_SEC in both the .cc and the .h file.
Sorry, it was silly of me not to realise that it was _parts_ of seconds that you were interested in, so you needed a double, with a decimal point. Applied. Thankyou.