Dan Smith wrote:
JG> # HG changeset patch
JG> # User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
JG> # Date 1194557358 18000
JG> # Node ID 94308147bed1693443d0741de6a30c5b0f77b0f1
JG> # Parent 543a0790d8615551153950de8f2f2fe3de107cf3
JG> Add Disk support to SettingsDefineCapabilities. Still lacks a bit of polish (i.e.
lack of warning when minimum or default is higher than maximum), but is functional.
I think this looks okay, but:
JG> +#define SDC_DISK_MIN 2000
JG> +#define SDC_DISK_DEF 5000
Why not make the increment amount also parameterized as such? Seems
like a valid thing to want to change. Unlikely, but valid :)
Sure, that's fine. With those two being defined as constants for disk
it's logical to define the last one as well, for organizational
purposes. That said, the next logical question could be, "Why aren't
the other numbers all defines as well?" My answer to that is, as a
habit I tend to only make things that are (or in this case, will be)
used in more than one place, defines. The way I see it, if I only use a
number once, having it as a define doesn't get me very far, because the
define really just creates a level of "indirection" for whomever decides
the value needs to be changed. As soon as the number is used twice, you
of course need a define so that nobody has to root out all instances of
the value, but for one-time-use-only values I don't generally bother
with defines.
This is a pretty good case for an exception to the rule, though, since
the lack of an INC define could be misleading.
--
-Jay