What is it with blank lines? - Electronic Circuits Projects Diagrams Free

Pommie · 4th February 2008, 03:28 PM

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,

Code:

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,

Code:

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 · 4th February 2008, 04:20 PM

Quote:

Originally Posted by Pommie

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 · 4th February 2008, 04:27 PM

I agree, your version is much easier to read.

Torben · 4th February 2008, 07:28 PM

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 · 4th February 2008, 10:37 PM

Quote:

Originally Posted by Torben

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 · 4th February 2008, 10:45 PM

My version;

Code:

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** · 4th February 2008, 11:09 PM

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.

Code:

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

The code is modified to be

Code:

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

when the following was intended

Code:

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

Had the original code been written as

Code:

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

the problem could have been avoided.

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

Pommie · 4th February 2008, 11:20 PM

3v0,

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

However, if someone wrote,

Code:

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

or, even worse,

Code:

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

Then I would probably miss that one.

Mike.

Torben · 4th February 2008, 11:32 PM

Quote:

Originally Posted by Brian Hoskins

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 · 4th February 2008, 11:41 PM

Quote:

Originally Posted by Brian Hoskins

My version;
<snip>

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

Code:

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 · 4th February 2008, 11:52 PM

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.

Code:

if(a==b){
    somert;
}

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

Code:

#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 · 4th February 2008, 11:53 PM

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

Torben · 5th February 2008, 12:00 AM

Quote:

Originally Posted by 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.

Code:

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.

Quote:

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 · 5th February 2008, 12:04 AM

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:

Code:

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 · 5th February 2008, 12:15 AM

Quote:

Originally Posted by aussiepoof

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

Quote:

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.

Quote:

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

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Similar Threads
Thread	Thread Starter	Forum	Replies	Latest
Magnets and Electromagnets	ElectroMaster	Electronic Theory	11	27th November 2007 02:31 PM
PIC code problem	Gaston	Micro Controllers	9	7th March 2007 02:23 AM
Charge batteries using tel lines	electroniks	Electronic Projects Design/Ideas/Reviews	5	16th April 2006 06:54 PM
running sort of output lines....................	anand_jain	Micro Controllers	1	11th July 2004 02:31 AM
TV- Lines in the picture	Johnson777717	General Electronics Chat	2	1st April 2004 12:10 AM

4th February 2008, 04:27 PM	(permalink)
Hero999	I agree, your version is much easier to read. __________________ I also post at the following sites: http://www.stop-microsoft.org http://www.heated-debates.com Screen name: Aloone_Jonez

4th February 2008, 07:28 PM	(permalink)
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.] Last edited by Torben; 4th February 2008 at 09:42 PM.

4th February 2008, 11:09 PM	(permalink)
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. Code: if (a>b) c = d; c++; The code is modified to be Code: if (a>b) c = d; a = p/2; c++; when the following was intended Code: if (a>b) { c = d; a = p/2; } c++; Had the original code been written as Code: if (a>b) { c = d; } c++; the problem could have been avoided. It is a pain but in the long run it pays. __________________ search engine for electronic parts Junebug USB PIC programmer kit., USB Bit Wacker, Homepage The 15 Minute Printed Circuit Board! (+drill time)

4th February 2008, 11:20 PM	(permalink)
Pommie	3v0, Surely, this is not a mistake that an experienced programmer would make. However, if someone wrote, Code: if (a>b); c = d; c++; or, even worse, Code: if (a>b); { c = d; } c++; Then I would probably miss that one. Mike.

4th February 2008, 11:53 PM	(permalink)
colin mac	http://www.gidnetwork.com/b-38.html

5th February 2008, 12:04 AM	(permalink)
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: Code: 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.