[ros-dev] Re: [ros-diffs] [gdalsnes] 18113: -reorder InsertXscendingOrder macro argument order and update uses

Casper Hornstrup ch at csh-consult.dk
Tue Sep 27 19:00:30 CEST 2005


> Hi,
> 
> IMHO, this is very bad code. You use a macro to hide a 'for' loop, and
> later in the source you use a 'break', 'continue' or 'goto' statement to
> leave the loop or to bypass some code.
> 
>         /* Find entry with SortKey greater than or equal to the
> passed-in SortKey */
>         LIST_FOR_EACH(ReturnEntry, &DeviceQueue->DeviceListHead,
> KDEVICE_QUEUE_ENTRY, DeviceListEntry)
>         {
>             /* Check if keys match */
>             if (ReturnEntry->SortKey >= SortKey) {
>                /* We found it, so just remove it */
>                RemoveEntryList(&ReturnEntry->DeviceListEntry);
>                break;
>             }
>         }
> 
> Such a source is simply unreadable.
> 
> - Hartmut

Yes, that is definitely one for the wall of shame ;-)

Casper



More information about the Ros-dev mailing list