What is it with blank lines?

[Pommie]

People keep posting code that has lots of blank lines. Why? It just causes me to scroll up/down in order to read it. I can understand one blank line separating routines for readabilities sake but every other line blank is just annoying.

Is it just me?

Mike.
Edit, this is in no way aimed at elec123. Sorry to use your code as an example.
P.S.
Here is what I mean.
Posted earlier,

void main(void);
void Initialize_ADC(void);


void main()
{

unsigned char dig_input;
unsigned char frank;

Initialize_ADC();

while(1)
{
//Read_ADC

ADGO = 1; //captures analogue voltage on pin.


frank = 0x10;
while(frank != 0x00)
{frank -= 1;}


while (ADGO)
{}

frank = 0xFF;
while(frank != 0x00)
{frank -= 1;}

dig_input = ADRESH; //defines the digital input.


if (dig_input<128)
{
CCPR1L = 0x00; //sets pwm to have duty cycle of zero.
CCP1CON = 0x00; 
}

else
{
dig_input -= 128; //dig_input = dig_input-128
dig_input += dig_input; //multiplies dig_input by 2

CCPR1L = look1[dig_input];
CCP1CON = look2[dig_input];

}

}
};

void Initialize_ADC()
{
TRISA = 0x01;
TRISC = 0x00;
PORTC = 0x00;
PR2 = 0x18; 
T2CON = 0x05;

ADCON0 = 0x80; //clear ADCON0 A2D starts sampling again.

//Set ADCON1

ADCON1 = 0x00; // clear ADCON1 register.//
ADCON1 = 0x02; // set PCFG1 bit i.e AN0 is analogue input.
ADON = 1;

};

My version,

void main(){
unsigned char dig_input;
unsigned int i;
    Initialize_ADC();
    While(1){
        //Read_ADC
        ADGO = 1; //captures analogue voltage on pin.
        while (ADGO);
        dig_input = ADRESH; //defines the digital input.
        if (dig_input<128){ //if solar panel (Vin) to boost converter is less than 10V don't boost.
            CCPR1L = 0x00; //sets pwm to have duty cycle of zero.
            CCP1CON = 0xc0; 
        }
        else{
            dig_input -= 128; //dig_input = dig_input-128
            dig_input += dig_input; //multiplies dig_input by 2
            CCPR1L = look1[dig_input];
            CCP1CON = look2[dig_input];
        }
    for(i=0;i<1000;i++);
    }
}

 

[futz]

Is it just me?

No, it's not just you. Some whitespace is very necessary as "punctuation" and to keep different sections of code separated visually. But too much of a good thing just sprawls the code unnecessarily long and you end up having to scroll up and down constantly to keep track of where it's going.

 

[Hero999]

I agree, your version is much easier to read.

 

[Torben]

Somewhere between the two extremes is good. The first code example is abysmal and if I saw that going into one of my codebases I'd have a little chat with the coder about the value of indenting (!) and overuse of vertical whitespace.

The second code example however offers no visual cues beyond the indenting as to what's going on and needs work on the horizontal whitespacing, but is still more readable than the first example (IMHO good indenting and consistent bracing trumps vertical whitespacing). For a little chunk of code like the example it's OK but I'd hate to manage a quarter-million line application written like that.

That said, discussions over code formatting often wind up in the same hundred-post arguments that vi vs. emacs or mac vs. windows do.


Torben

[Edit: I just saw the post with the original code. I take back my rating of "abysmal" on the original code since it wasn't posted in a code block, which screwed up the formatting. I do agree it's a bit heavy on the vertical whitespace though.]

 

[Brian Hoskins]

That said, discussions over code formatting often wind up in the same hundred-post arguments that vi vs. emacs or mac vs. windows do.

People have even written books on it, for goodness sake. Sod that is my opinion - I'll keep guidelines in mind and I'm always happy to hear advice, but basically I'll pick a code writing method that I personally feel is the most sensible and stick to that.

Brian

 

[Brian Hoskins]

My version;

void main()
{
unsigned char dig_input;
unsigned int i;
    
Initialize_ADC();
While(1)
    {
    //Read_ADC
    ADGO = 1; //captures analogue voltage on pin.
    while (ADGO);
    dig_input = ADRESH; //defines the digital input.
    if (dig_input<128){ //if solar panel (Vin) to boost converter is less than              10V don't boost.
    CCPR1L = 0x00; //sets pwm to have duty cycle of zero.
    CCP1CON = 0xc0; 
    }
else
    {
    dig_input -= 128; //dig_input = dig_input-128
    dig_input += dig_input; //multiplies dig_input by 2
    CCPR1L = look1[dig_input];
    CCP1CON = look2[dig_input];
    }

for(i=0;i<1000;i++);
    
}

 

[3v0]

Style is (or should be) about readability and maintainability.
As Brian said there are books on the subject of style. Even more important are the books on writting "correct" code.

Back when we used ASCII terminals we needed to crunch the code a bit but that caused problems.

Many man years have been spent tracking down missing "{" or "}". For that reason it makes sense to put the opening and closing codeblock char at the same indent level and as the first character on a line.

Another gotcha is the if without a code block.

if (a>b)
    c = d;
c++;

The code is modified to be

if (a>b)
    c = d;
    a = p/2;
c++;

when the following was intended

if (a>b)
{
    c = d;
    a = p/2;
}
c++;

Had the original code been written as

if (a>b)
{
    c = d;
}
c++;

the problem could have been avoided.

It is a pain but in the long run it pays.

 

[Pommie]

3v0,

Surely, this is not a mistake that an experienced programmer would make.

However, if someone wrote,

if (a>b);
    c = d;
c++;

or, even worse,

if (a>b);
{
    c = d;
}
c++;

Then I would probably miss that one.

Mike.

 

[Torben]

People have even written books on it, for goodness sake. Sod that is my opinion - I'll keep guidelines in mind and I'm always happy to hear advice, but basically I'll pick a code writing method that I personally feel is the most sensible and stick to that.

Brian

Yep, I agree. As long as a project is self-consistent I'm usually happy. And it helps if it's a fairly common style so that people using different editors can set it up easily if the editor supports that. Ultimately I don't really care whether I'm using K&R or GNU or Linux kernel style or whatever as long as it's the same throughout a codebase and everybody's aware of it.


Torben

 

[Torben]

My version;
<snip>

Just for the heck of it, this is how I would typically write that:

void main()
{
    unsigned char dig_input;
    unsigned int i;

    Initialize_ADC();

    while (1)
    {
        // Read_ADC                                                                                                                                                                                        
        ADGO = 1; // captures analogue voltage on pin.                                                                                                                                                     
        while (ADGO) {}
        dig_input = ADRESH; // defines the digital input.                                                                                                                                                  

        // if solar panel (Vin) to boost converter is less than 10V don't boost.                                                                                                                           
        if (dig_input < 128)
        {
            CCPR1L = 0x00; // sets pwm to have duty cycle of zero.                                                                                                                                         
            CCP1CON = 0xc0;
        }
        else
        {
            dig_input -= 128; // dig_input = dig_input-128                                                                                                                                                 
            dig_input += dig_input; // multiplies dig_input by 2                                                                                                                                           
            CCPR1L = look1[dig_input];
            CCP1CON = look2[dig_input];
        }

        for (i = 0; i < 1000; i++) {}
    }
}

Not too different but I at least find it slightly easier to read--but the differences are minimal at best.


Torben

 

[Pommie]

I don't have a problem with any of the above examples. Looking at them, it appears my use of an open brace on the end of the line is frowned upon.
I.E.

if(a==b){
    somert;
}

The vertical spacing is not the only problem. This from another recent post,
(my apologies to the original poster.)

#pragma romdata


//Example heirarchy items.

// **********************************************************************************************
// The following arrays contain menu items for each level of menu heirarchy.
MenuItem menu1Items[] =
						{
						 // TopLevel    , Name     ,   subMenu, handler
							1			,"Menu1-1" , 0		 ,  handler,
							1			,"Menu1-2" , 0		 ,  0
							
						};


//This represents the settings sub menu.
MenuItem menu2Items[] =
						{
						 // TopLevel    , Name     ,   subMenu, handler
							1			,"Menu2-1" , 0 		 ,  0,
							1			,"Menu2-2" , 0		 ,  0,
							1			,"Menu2-3" , 0		 ,  0
							
						};
						
						
//base items. also: menu initialises by cha startup item to baseItems[0]
MenuItem menu0Items[] =
						{
						 // TopLevel    , Name     ,   subMenu, handler
							0           , "Menu1"  ,  &menuLevels[1] , 0,
							0           , "Menu2"  ,  &menuLevels[2] , 0,	
							1           , "Menu3"  ,  0 , 0	
							
						};
						
						
//*********************************************************************************************
//End of item arrays



//The following array is required by menu system. 
//It contains MenuLevel items, which describe a level that contains menu items.
//First item should be the base menu.
MenuLevel menuLevels[] =	{			// start index ,	array size 							  , array
										 0		   	   ,	sizeof(menu0Items) / sizeof (MenuItem), menu0Items,
										 0		   	   ,	sizeof(menu1Items) / sizeof (MenuItem), menu1Items,
										 0		   	   ,	sizeof(menu2Items) / sizeof (MenuItem), menu2Items,
									 
								
							};

Mike.

 

[colin mac]

http://www.gidnetwork.com/b-38.html

 

[Torben]

I don't have a problem with any of the above examples. Looking at them, it appears my use of an open brace on the end of the line is frowned upon.
I.E.

if(a==b){
    somert;
}

Not at all; I just tend to code in what Wikipedia calls Allman style. The open-brace-on-control-statement-line style above is typical of K&R and some other styles. I like that one too.

The vertical spacing is not the only problem. This from another recent post,
(my apologies to the original poster.)
<snip>
Mike.

Yeah, I don't like that. Hard to maintain and its visual layout depends too much on the editor's setup.


Torben

 

[aussiepoof]

The great thing about this topic is that *everyone* has an opinion and some of us are so indecisive we have two

Here's my take:

void main()
{
    unsigned char dig_input;
    unsigned int i;
    
    Initialize_ADC();
    while(1)
    {
        //Read_ADC
        ADGO = 1; //captures analogue voltage on pin.
        while (ADGO);
        dig_input = ADRESH; //defines the digital input.
        // Boost when (Vin) to boost converter is less than 10V (128)
        if (dig_input<128)
        {
             CCPR1L = 0x00; //sets pwm to have duty cycle of zero.
             CCP1CON = 0xc0; 
        }
        else
        {
            dig_input -= 128; // dig_input = dig_input-128
            dig_input += dig_input; // multiplies dig_input by 2
            CCPR1L = look1[dig_input];
            CCP1CON = look2[dig_input];
        }
        for(i=0;i<1000;i++);
    }
} // main()

This formatting allowed me to easily find the missing brace in Brian Hoskin's code posting - and no, I'm not impugning Brian, just pointing out why formatting is so important.

BTW: I improved the comment on the first if(), but the comments on the manipulations of dig_input leave a lot to be desired... comments ought to refer to the logic of the solution, not merely be a reiteration in English (or language of choice) of the code!!!

I always put a comment on the closing brace of a function as above so I can navigate my code a little more easily.

P.

 

[Torben]

The great thing about this topic is that *everyone* has an opinion and some of us are so indecisive we have two

This formatting allowed me to easily find the missing brace in Brian Hoskin's code posting - and no, I'm not impugning Brian, just pointing out why formatting is so important.

I spotted that too but figured it a was copy-and-paste error, but it's a good point. When I hit auto-indent in emacs and it gets confused, I know there's some kind of error and can more easily track it down.

BTW: I improved the comment on the first if(), but the comments on the manipulations of dig_input leave a lot to be desired... comments ought to refer to the logic of the solution, not merely be a reiteration in English (or language of choice) of the code!!!

Agreed. If the code is not meant to be instructional, then I prefer to see functional commenting as opposed to descriptive, meaning the comments should generally state *what* the code is supposed to be doing, not *how*.

Regexps (and perl ) are obvious exceptions to the above, as they can lose even an experienced reader pretty quickly if they're complex.


Torben

 

[3v0]

3v0,

Surely, this is not a mistake that an experienced programmer would make.

...

Mike.

Maybe not when fresh, but many people work past the point where they should knock it off. All the examples here have been quite short. A few hundred or thousand lines of code is quite different. It is best to do what one can to avoided problems.

 

[Torben]

Many people work past the point where they should knock it off. All the examples here have been quite short. A few hundred or thousand lines of code is quite different. It is best to do what one can to avoided problems.

I agree with that one too. I've got a 230,000 line project and when I get new coders on board it's "Yes, I know it's slightly tedious for the one-liners. Do it anyway."


Torben

 

[donniedj]

This is partly why I dislike C, difficult to read, susceptible to too much interpretation, and the written text becomes more effort consuming than the original reason for the code. It should solve problems and not add to it.

 

[Torben]

This is partly why I dislike C, difficult to read, susceptible to too much interpretation, and the written text becomes more effort consuming than the original reason for the code. It should solve problems and not add to it.

You assume we're talking about C (probably because I'm going on about K&R and the examples so far have been C) but this stuff applies to any language (er. . .except maybe this one). The actual semantics differ but the generalities really don't.

A good programmer can program well no matter the language; a bad programmer sucks just as hard in an academically "good" language as in any other. A bad programmer will, I freely admit, have more trouble with string handling and pointers in C than in languages which shield them from having to know what's really going on behind the scenes, though.

I'm not sure how C is "susceptible to interpretation", anyway. The code does what it says it does. Whether that's what the programmer intended is another question.

I like C for certain problems. Others demand assembly, some are solved easiest in PHP, and others just cry out for a lisp-like language. Good commenting and consistent structure are important in all.

Just my $0.02 CDN.


Torben

 

[3v0]

Torben nailed it.

I would add that the proper choice of names and their scope is often overlooked. It is very important in writing good code. And not always easy to do.