[Libvirt-cim] [PATCH 3 of 6] 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

[Jay Gagnon]

Dan Smith wrote:
> JG> # HG changeset patch
> JG> # User Jay Gagnon <grendel at 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