From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Macros for time magic values |
Date: | 2011-03-14 16:36:36 |
Message-ID: | 201103141636.p2EGaap26515@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert Haas wrote:
> On Mon, Mar 14, 2011 at 12:01 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> >> My first reaction that this change was about a net wash in
> >> readability for me -- in a couple places it might save me a few
> >> moments thinking about what the number was meant to represent,
> >> balanced against following the ctag back to the #define to see what
> >> number was used for things like DAYS_PER_YEAR or DAYS_PER_MONTH.
> >
> >> Comments like the one Bruce cites above seem like they tip the
> >> scales in favor of the patch for me. ?Having a place to document
> >> the choice of questionable values seems like it's better than just
> >> using the questionable values "bare" all over the place. ?Neither
> >> omission of the justification nor repeating it seems better.
> >
> > Another advantage of the macros is that it makes it a lot easier to grep
> > to see where a questionable value is being used. ?Originally I'd felt
> > that wrapping those bogus numbers in macros was a bad idea, but the
> > documentation and searching advantages are enough to make me think it's
> > all right.
>
> Yeah, I agree. And I do think that there is also some value of having
> constants for SECS_PER_MINUTE and MINUTES_PER_HOUR, because otherwise
> it can be unclear what 60 means in a particular context. We're at
> the high end of what I consider reasonable in terms of defining
> constants to represent values that aren't likely to change, but there
> is tangible value in being able to grep for those constants when
> you're trying to figure out what things might need changing, or just
> to understand the code better.
Yes, I did have to study the code to figure out which to use:
if (type == TZ || type == DTZ)
{
tz = -(val * MINS_PER_HOUR);
result = dt2local(timestamp, tz);
}
We measure timezone differences in minutes.
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2011-03-14 16:42:58 | Re: Macros for time magic values |
Previous Message | Merlin Moncure | 2011-03-14 16:35:43 | Re: Avoiding repeated ON COMMIT truncation for temporary tables |