[Freeciv-Dev] Re: Freeciv commit: vas: Added vector iterator macro, append and get the...

[Vasco Alexandre Da Silva Costa <vasc@xxxxxxxxxxxxxx>]

On Wed, 19 May 2004, Raimar Falke wrote:

> On Wed, May 19, 2004 at 01:20:44PM -0700, Raimar Falke wrote:
> > -  if (index < 0 || index >= tthis->size) {
> > -    assert(index >= 0 && index < tthis->size);
> > -    return NULL;
> > +  if (index == -1) {
> > +    if (tthis->size > 0) {
> > +      return tthis->p + tthis->size - 1;
> > +    } else {
> > +      return NULL;
> > +    }
> >    } else {
> > -    return tthis->p + index;
> > +    if (index < 0 || index >= tthis->size) {
> > +      return NULL;
> > +    } else {
> > +      return tthis->p + index;
> > +    }
>
> What about this:
>
>   if (index == -1 && tthis->size > 0) {
>     return tthis->p + tthis->size - 1;
>   } else if (index >= 0 && index < tthis->size) {
>     return tthis->p + index;
>   } else {
>     return NULL;
>   }
>
> Less redundant, more compact and with a clean else case.
>
> > +static inline void SPECVEC_FOO(_vector_append) (SPECVEC_VECTOR *tthis,
> > +                                           SPECVEC_TYPE *pfoo)
> > +{
> > +  const size_t last = tthis->size;
> > +
> > +  SPECVEC_FOO(_vector_reserve) (tthis, last + 1);
> > +  tthis->p[last] = *pfoo;
> > +}
>
> I think the variable is misnamed here. Because of the reserve call the
> meaning changes from "size" to "index of last element".
>
> What about the classical:
>
> {
>   SPECVEC_FOO(_vector_reserve) (tthis, tthis->size + 1);
>   tthis->p[tthis->size - 1] = *pfoo;
> }

Looks nice. Why don't you make a patch? You already did the hard work. :-)

---
Vasco Alexandre da Silva Costa @ Instituto Superior Tecnico, Lisboa