Indent

[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 )

[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

[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

[-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.

[-m32]

Code: Select all

if(strcmp(info_block->signature, "PMID") == 0) {

Might want to use strncmp

[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?

[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

Might want to use strncmp

Doh, didn't think of that

[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'
   )
{
    ...
}

[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