Rockbox.org home
release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Rockbox mail archive

Subject: Re: Patch #706111 id3 tags, implemented! But an error appears!
From: Björn Stenberg (bjorn_at_haxx.se)
Date: 2003-06-05


TP Diffenbach wrote:
> But changing 'sizeof( "TCON" ) - 1' to that expression's value, '4', is I
> think a loss. '4' by itself doesn't document anything, and certainly doesn't
> indicate to the reader what the number is used for. By showing the
> expression from which 4 results, the user knows that it relies on the length
> of the string "TCON" less the nul terminator, and is not something else
> entirely.

The name of the field is 'tag_length', which I think fully communicates the purpose of it. I removed the sizeof() construct simply because I found it made the code unneccessarily messy.

> While not making it impossible to make errors, this I think makes it easier
> for coders adding tags, by relegating tediously repeated code to the macro.

I dislike macros that hide code. It is in my opinion mostly better to have slightly more complex code if it avoids having to pre-process it in your head.

I also do not like to create language constructs to protect the programmer from himself. Errors will always happen, but they are much simpler to find and fix if the code just does the job straight with as little fuss as possible.

-- 
Björn



Page was last modified "Jan 10 2012" The Rockbox Crew
aaa