Page 1 of 1
Indent
Posted: Thu Aug 07, 2008 12:47 pm
by Adek336
My code is ugly as night.
Code: Select all
if (info_block->signature[0]=='P' && info_block->signature[1]== 'M' &&
info_block->signature[2]=='I' &&
info_block->signature[3]=='D') {
uint8 suma=0;
for (int i= 0; i<sizeof(struct Vesa3PMInfoBlock); i++)
suma += *(ptr + i);
if (suma == 0) {
println((uint32)ptr);
return info_block;
}
How can I make it more readable? Especially the long if-clause, how can I make it look prettier? ( but I don't want a info_block->signature32 == 0xdeadbeef al-4-bytes-at-once-checked because I don't like it that way
)
Re: Indent
Posted: Thu Aug 07, 2008 1:23 pm
by xyzzy
Code: Select all
if(strcmp(info_block->signature, "PMID") == 0) {
Assuming you have a strcmp function. Probably not all people would like doing it with strcmp, but that's how I would do it. Also, I'd recommend wrapping that 1-line for loop in braces, even if it is just one line. I've had quite a few bugs where I thought a line was in a for loop when it actually wasn't because I didn't notice the lack of braces, which is why I always use braces now
Re: Indent
Posted: Thu Aug 07, 2008 1:53 pm
by AJ
Hi,
If you want to generally improve your code readability, have a look
-> here <-, which is the style document I go by.
Cheers,
Adam
Re: Indent
Posted: Thu Aug 07, 2008 8:02 pm
by -m32
Mine simply does this:
Code: Select all
if(vbios[i] == 'P' && memcmp(&vbios[i], "PMID", 4) == 0) {
That way it only calls memcmp if the first character is a P instead of every time through the loop.
Re: Indent
Posted: Thu Aug 07, 2008 8:04 pm
by -m32
AlexExtreme wrote:Code: Select all
if(strcmp(info_block->signature, "PMID") == 0) {
Might want to use strncmp
Re: Indent
Posted: Thu Aug 07, 2008 10:24 pm
by Adek336
Nice ones:) but how should I redact it if I had to use the orignal condition
Code: Select all
if (info_block->signature[0]=='P' && info_block->signature[1]== 'M' &&
info_block->signature[2]=='I' &&
info_block->signature[3]=='D') {
uint8 suma=0;
Code: Select all
if (info_block->signature[0]=='P' && info_block->signature[1]== 'M' && info_block->signature[2]=='I' && info_block->signature[3]=='D') {
uint8 suma=0;
Code: Select all
if (info_block->signature[0]=='P' && info_block->signature[1]== 'M' && info_block->signature[2]=='I' && info_block->signature[3]=='D')
{
uint8 suma=0;
Code: Select all
if (
info_block->signature[0]=='P' &&
info_block->signature[1]== 'M' &&
info_block->signature[2]=='I' &&
info_block->signature[3]=='D'
) {
uint8 suma=0;
etc?
Re: Indent
Posted: Fri Aug 08, 2008 12:37 am
by xyzzy
My personal style would be this:
Code: Select all
if (info_block->signature[0]=='P' && info_block->signature[1]== 'M' &&
info_block->signature[2]=='I' &&
info_block->signature[3]=='D') {
uint8 suma=0;
The extra lines of the condition are indented by 2 rather than 1 so that they do not get confused with the actual code inside the if block. Of course where possible I try to avoid large conditions like that
-m32 wrote:
Might want to use strncmp
Doh, didn't think of that
Re: Indent
Posted: Fri Aug 08, 2008 12:39 am
by Solar
strcmp() would be wrong here, as "PMID" is not zero-terminated. strncmp() does the trick. I don't see how -m32's memcmp()-only-if-first-character-matches should help performance.
As for indenting, I'd write:
Code: Select all
if ( info_block->signature[0] == 'P' &&
info_block->signature[1] == 'M' &&
info_block->signature[2] == 'I' &&
info_block->signature[3] == 'D'
)
{
...
}
Re: Indent
Posted: Fri Aug 08, 2008 1:07 am
by Adek336
Solar, I like the way that parenthesis has its own line
seems to be what I was looking for
However, there doesn't seem to be much choice
I thought there would be like thousands of differents ways to redact it!
One thing I found unpleasant from time to time is how with this indentation
if ( belzebub kontrabant megahum >>>> x form megarangers commit svn eyes sewn shut &&
necrophallus vomitorium phobophille lol fag ( x[] doom shall rise sunn O)))) love will tear us apart)
{
y = namespaceGenerator:
:tele::market( PTE_HI | PTE_HELLO, STRAWBERRY | JELLY, 0x3303);
where the condition and the instruction are of similar lengths, you can't immiedately tell they're not both parts of the condition
the parenthesis idea helps a bit
AJ, terrific link, thanks
:tele::market( PTE_HI | PTE_HELLO, STRAWBERRY | JELLY, 0x3303);