[GRASS-dev] lib fn to convert int color number to RGB values?

[Glynn Clements]

Ivan Shmakov wrote:

>  >> Is a new D_number_to_RGB(int color, unsigned char &r,&g,&b) fn
>  >> needed in lib/display/tran_colr.c?
> 
>  > Yes. E.g.:
> 
>  > int D_color_number_to_RGB(int color, int &r, int &g, int &b)
>  > {
> 
> 	That looks a lot like C++.  Surely it will work with C89?

Oops. I meant:

	int D_color_number_to_RGB(int color, int *r, int *g, int *b)

>  >     const struct color_rgb *c;
> 
>  >     if (color <= 0)
>  >         return 0;
> 
> 	BTW, is it a GRASS convention to use 0 to indicate failure?
> 	Standard C library functions tend to use -1 for failure, while
> 	using non-negative values to indicate success.

The GRASS convention is to use a different convention for each
function ;)

More seriously: functions which only return a success/failure
indication typically (but not always) return 1 for success and 0 for
failure, so that conditionals read naturally, i.e. "if (foo(...))" 
means "if it succeeds" and "if (!foo(...))" means "if it fails".

Functions which normally return a non-negative integer normally return
a negative integer to indicate failure.

Functions which return 0 for success should ideally be tested using
"if (foo(...) == 0)" rather than "if (!foo(...))", as the use of "!" 
connotes a negative (i.e. failure).

> 	Also, it seems feasible to set `errno' in the library functions
> 	like this.  Do I understand correctly that it isn't done (with
> 	any regularity) in GRASS?

AFAIK, GRASS itself never sets errno.

>  >     if (color >= ncolors)
>  >         return 0;
> 
>  >     c = &colors[color];
>  >     r = c->r;
>  >     g = c->g;
>  >     b = c->b;
> 
>  >     return 1;
>  > }
> 
> [...]
> 
> 	I'd suggest the following, but I don't know (yet) the
> 	appropriate conventions for the D_* functions.
> 
> int
> D_color_number_to_RGB (int color, int *r, int *g, int *b)
> {
>     const struct color_rgb *c;
> 
>     if (color <= 0 || color >= ncolors) {
> 	errno = EINVAL;
>         return -1;
>     }
> 
>     c = &colors[color];
>     if (r != 0) *r = c->r;
>     if (g != 0) *g = c->g;
>     if (b != 0) *b = c->b;
> 
>     return 0;
> }
> 
> 	With the `if (X != 0)' guards, the function may be used to query
> 	whether the given color index exists, while not asking for its
> 	components to be returned, like:
> 
>    if (D_color_number_to_RGB (INDEX, 0, 0, 0) < 0) {
>        /* color INDEX doesn't exist */
>    }

That makes sense, although I would omit the "!= 0", and just use e.g. 
"if (r) ...". Generally, I prefer "if (x)" and "if (!x)" if zero/NULL
is a false/unset/etc indicator.

> 	If `struct color_rgb' is a ``public'' data type, then it might
> 	be appropriate to code this function as:

The problem is that we have both color_rgb (colors.h) and RGBA_Color
(gis.h) structures, and we should deprecate one of them (probably
color_rgb).

-- 
Glynn Clements <glynn at gclements.plus.com>