Help required
akshay...@gmail.com
I am a newbie in the field of microcontrollers and embedded
programming. I am trying to implement a basic version of the hangman
game as a project for my course. (I have to use 8051)
I am using RIDE software for coding in C. I only have a rough idea as
to how i should go about writing the code. By referring to the
examples, I tried to just declare my functions and my main program. I
am yet to define the functions and I am not sure that what I have done
so far is correct.
Could you look into it and tell me whether I am doing alright as of now
or am I hopelessly lost?
Thanks!
Akshay
char code word1[] = {'A','B','S','O','L','U','T','E'}; //WORD TO BE
GUESSED
char code table1[] = {'A','B','C','D','E','F','G','H','I'}; //LOOKUP
TABLE1 FOR KEYPAD
char code table2[] = {'J','K','L','M','N','O','P','Q','R'}; //LOOKUP
TABLE2 FOR KEYPAD
char code table3[] = {'S','T','U','V','W','X','Y','Z'}; //LOOKUP
TABLE3 FOR KEYPAD
void LCD_INIT(void){}//ROUTINE TO INITIALIZE LCD WITH 8 UNDERSCORES
char GET_CHAR(void){ //KEYPAD ROUTINE TO GET A CHARACTER
/*This routine will get two characters from the keypad and determine
which alphabet corresponds to the combination
*/
}
void LCD_DISPLAY(char c,int i){} //LCD ROUTINE TO DISPLAY A CHAR IN
POSITION i
void DISPLAY_MESG(char* str){} //LCD ROUTINE TO DISPLAY A MESSAGE IN
LINE 2
void LEDDISP(int i){} //TO SWITCH ON THE i'th LED
void main(void){
data char keypressed; //A BYTE IN THE RAM FROM 30H TO 7FH
int i,ccnt=0,wcnt=0;
char found='N';
P2=255; //P2 IS FOR INPUT FOR KEYPAD COLUMNS
LCD_INIT();
while(1){
keypressed=GET_CHAR();
i=0;
while(i<8){
if(word1[i]==keypressed){
LCD_DISPLAY(keypressed,i);
found='Y';
ccnt++;
if(ccnt==8){ //CHECK IF 8 CHARS HAVE BEEN GUESSED
DISPLAY_MESG("Congrats");
goto out;
}
}
i++;
}
if(found=='N'){
wcnt++;
LEDDISP(wcnt-1);
if(wcnt==4){ //CHECK IF THERE HAVE BEEN 4 WRONG GUESSES
DISPLAY_MESG("Game Over");
goto out;
}
}
}
out:while(1);
}
Gene S. Berkowitz
akshay...@gmail.com says...
> Here is the code:
>
> #include<reg51.h>
<snip>
Great. Now you've guaranteed that the OP will not actually LEARN
anything.
--Gene
Pete Fenelon
That was the OP following up (with the program he's copied from another
student, probably).
pete
--
pe...@fenelon.com [Support no2id.net: working to destroy Blair's ID card fraud]
John B
If he writes C code with "goto" in it he's not capable of learning
anything anyway.
--
John B
larwe
John B wrote:
> If he writes C code with "goto" in it he's not capable of learning
> anything anyway.
You haven't lived until you've debugged code like this (there are NO
errors on my part in this code, it's pseudo representation of ACTUAL
code I've been called upon to fix in the past):
int func(int param)
{
if (param)
do_something((int) do_something_else);
else if (param)
do_something_else();
}
int do_something (int param) {
if (param == 1)
do_something_else();
label:
asm {
mov a,#1
call do_something_else
jmp do_something
}
}
int do_something_else(void) {
if (global_variable) goto label;
else asm {
ret;
}
}
Just what the hell this code was supposed to do, your guess is as good
as mine. I lost track trying to trace through just how it ever exited
given that you have C calling assembly and stack frames appearing and
disappearing all over the place.
Paul Keinanen
<spamj_ba...@blockerzetnet.co.uk> wrote:
>If he writes C code with "goto" in it he's not capable of learning
>anything anyway.
IMHO, the use of goto is quite appropriate in this case, since C only
supports exiting the innermost loop with break. A selective use of
goto can often improve code readability and reduces the level of
indent required.
Some seems to fear the "horrible" word goto so much that they rather
use complex flags to control loop execution etc. making programs very
hard to read.
Paul
Isaac Bosompem
What the hell is that code supposed to do? That is just downright nasty
way of coding. I don't know if the person who wrote that was still
working there when you were called upon to fixed the code, I would
suggest that he or she go to some seminar at least to avoid a problem
like that again!!
-Isaac
Akshay
am sorry if my code is terrible.Which is why I am asking for help.
I am trying to interafce LCD along with keypad along with 8051 to
implement the hangman game. I am using RIDE. And this is as far as I
have got.(after going through the examples).
word1[] is the word to be used in the game.
tables1,2,3 are lookup tables for the hex keypad.
This code WILL not work as I have only declared most of my subroutines.
I am getting a character from the user, checking whether it is there in
the word, if so displaying it in the coorect position in the LCD. I
also check whether the number of correct guesses equals 8(length of
word). If so I display "Congrats"
Else, I increment the number of wrong guesses and check whether it
equals 4. If so I display "Game Over"
If any of you have suggestions or advice, I am ready to listen
I posted here only to get help, not to get flamed.
larwe
Isaac Bosompem wrote:
> > You haven't lived until you've debugged code like this (there are NO
> > errors on my part in this code, it's pseudo representation of ACTUAL
> > code I've been called upon to fix in the past):
>
> What the hell is that code supposed to do? That is just downright nasty
I'm still not entirely sure. I mean, what do you do - besides maybe
resign - when you see a piece of code with comments and structure like
this:
if (flags && FLAG_1) // is FLAG_1 clear?
{
do_something();
// select thing to do based on FLAG_1
if (flags & FLAG_1)
do_something_else();
else if (flags & FLAG_1)
do_something_different();
special_setting = 0x80; // uint8_t type
special_setting = special_setting << 1; // multiply by 2 for louder
output
// do things 123 times
for (i=0;i<123;i++);
do_things();
}
(those errors are not typos - at least not on MY part). And since there
is no written specification for the product, the only way to "debug" it
is to go to the QA department and ask them to demonstrate how a similar
product ("gold standard") that made it through QA some years ago
behaves.
You then go back to your computer armed with some guesses as to what a
few of the comments mean, and perhaps some info from the QA tech about
the history of a few behaviors, to give you a clue as to the cryptic
tangle of special-case code.
Timers never initialized. Timers initialized with magic numbers based
on obsolete clock module settings, leading to bizarre race conditions
and out-of-spec communications. Watchdogs turned off with comments that
the unit inevitably resets if the WDT is enabled. A structure that's 32
bytes long being written to nonvolatile memory with code like this:
write_eeprom(&structure, 67); // structure is 67 bytes long
and worse, read back to RAM with:
read_eeprom(&structure, 117); // structure is 117 bytes long
(as it happens, by amazing total whacked-out luck everything in the
100-odd bytes after "structure" in RAM just happens to be noncritical).
Oh, and I forgot to mention that "structure" is not actually a
structure, it is just a pointer to a variable named, say, fred - the
code RELIES ON THE FACT that fred is followed in RAM by a few other
variables that need to be saved to EEPROM - never mind the fact that
they are all in different object files and there's no explicit linker
setting made to put those variables in the right place (and not even a
comment in the source saying "don't add variables here"). Probably the
67 and 117 "sizes" were calculated by the difference in address between
(first variable of interest) and (last variable of interest) in some
specific build(s).
I could go on and on like this for hours. I never thought of the
logical operators in C as weapons of mass destruction, but this
person's coding proves it to be the case. People have been executed for
lesser offenses than all this. The thing you don't see is that 80% of
these errors are buried in code paths that never get executed because
of code like this:
if (!something)
{
do_stuff();
something = FALSE;
if (something) { // doubly impossible to reach this point
do_bad_things();
}
}
> way of coding. I don't know if the person who wrote that was still
> working there when you were called upon to fixed the code, I would
Yes. But that person can't answer the question either. It needs more
than a seminar to fix; I think electroconvulsive therapy might be the
answer.
Isaac Bosompem
Well Akshay, I was not referring to your code. I was referring to the
code Iarwe posted.
Your code looks decent and is readable.
The genral consensus of C pro's though is to avoid using goto. To tell
you the truth, easier said than done in some specific cases.
-Isaac
Vadim Barshaw
> [...] And since there
> is no written specification for the product, the only way to "debug" it
> is to go to the QA department and ask them to demonstrate how a similar
> product ("gold standard") that made it through QA some years ago
> behaves.
If they have the QA department, they are supposed to have code reviews, aren't they? If they don't have code reviews, what 'Q' stands for than? I know, the questions are rhetorical :)
I saw the code review, where the reviewee had left a section of code surrounded with "#if 0 / #endif" pair. The coding standard (yes, there was one) explicitly prohibited such (ab)use of preprocessor. The reviewer had to perform a lecture that "#if 0" could not be used. The next bunch of diffs contained, among others, the following wonderful lines:
137c137
< #if 0
---
> #if 1
Isaac Bosompem
larwe wrote:
> Isaac Bosompem wrote:
>
> > > You haven't lived until you've debugged code like this (there are NO
> > > errors on my part in this code, it's pseudo representation of ACTUAL
> > > code I've been called upon to fix in the past):
> >
> > What the hell is that code supposed to do? That is just downright nasty
>
> I'm still not entirely sure. I mean, what do you do - besides maybe
> resign - when you see a piece of code with comments and structure like
> this:
>
> if (flags && FLAG_1) // is FLAG_1 clear?
> {
> do_something();
>
> // select thing to do based on FLAG_1
> if (flags & FLAG_1)
> do_something_else();
> else if (flags & FLAG_1)
> do_something_different();
>
> special_setting = 0x80; // uint8_t type
> special_setting = special_setting << 1; // multiply by 2 for louder
> output
>
> // do things 123 times
> for (i=0;i<123;i++);
> do_things();
> }
Reminds me of some C code on the internet that I saw in which someone
decided to use a base 2 log to determine if a bit in a variable was
set!! I mean, why do these complicated things if there is a much
easier, simpler (and MUCH faster) way to accomplish it?
> (those errors are not typos - at least not on MY part). And since there
> is no written specification for the product, the only way to "debug" it
> is to go to the QA department and ask them to demonstrate how a similar
> product ("gold standard") that made it through QA some years ago
> behaves.
>
> You then go back to your computer armed with some guesses as to what a
> few of the comments mean, and perhaps some info from the QA tech about
> the history of a few behaviors, to give you a clue as to the cryptic
> tangle of special-case code.
>
> Timers never initialized. Timers initialized with magic numbers based
> on obsolete clock module settings, leading to bizarre race conditions
> and out-of-spec communications. Watchdogs turned off with comments that
> the unit inevitably resets if the WDT is enabled. A structure that's 32
> bytes long being written to nonvolatile memory with code like this:
yikes, I never knew anyone that would not use a device simply because
it served its purpose.
> write_eeprom(&structure, 67); // structure is 67 bytes long
> and worse, read back to RAM with:
> read_eeprom(&structure, 117); // structure is 117 bytes long
>
> (as it happens, by amazing total whacked-out luck everything in the
> 100-odd bytes after "structure" in RAM just happens to be noncritical).
Again, you'd think people would not do things like this to make
debugging easy. I just cannot imagine why something like you described
above would be a good idea.
> Oh, and I forgot to mention that "structure" is not actually a
> structure, it is just a pointer to a variable named, say, fred - the
> code RELIES ON THE FACT that fred is followed in RAM by a few other
> variables that need to be saved to EEPROM - never mind the fact that
> they are all in different object files and there's no explicit linker
> setting made to put those variables in the right place (and not even a
> comment in the source saying "don't add variables here"). Probably the
> 67 and 117 "sizes" were calculated by the difference in address between
> (first variable of interest) and (last variable of interest) in some
> specific build(s).
Reminds me of some Amiga based code I was reading about in which a demo
programmer (using I think a loader) overwrote some critical OS data
structure and still managed to run without a hitch!! Perhaps he should
take a shot at the slot machines ;).
> I could go on and on like this for hours. I never thought of the
> logical operators in C as weapons of mass destruction, but this
> person's coding proves it to be the case. People have been executed for
> lesser offenses than all this. The thing you don't see is that 80% of
> these errors are buried in code paths that never get executed because
> of code like this:
>
> if (!something)
> {
> do_stuff();
> something = FALSE;
> if (something) { // doubly impossible to reach this point
> do_bad_things();
> }
> }
You'd think that the developers working on this stuff would go out of
their way to make debugging the firmware of these products much simpler
and easier, rather than obfuscating simple logic testing structures
like that!
How do they debug code like that?!
> > way of coding. I don't know if the person who wrote that was still
> > working there when you were called upon to fixed the code, I would
>
> Yes. But that person can't answer the question either. It needs more
> than a seminar to fix; I think electroconvulsive therapy might be the
> answer.
hahaha, It might do everyone else some good too.
Also Akshay, you can basically take the examples Iarwe posted as
pitfalls to avoid when coding for an embedded system. Not only will you
save yourself headaches and stress, you will have a lot more fun doing
your project :).
-Isaac
larwe
Vadim Barshaw wrote:
> > is no written specification for the product, the only way to "debug" it
> > is to go to the QA department and ask them to demonstrate how a similar
>
> If they have the QA department, they are supposed to have code reviews, aren't they?
> If they don't have code reviews, what 'Q' stands for than? I know, the questions are rhetorical :)
Actually there are "good" answers to these questions. This code
predates our current ISO procedures. At the time it went through, I
believe the official procedure was "test it in engineering, throw it
over the wall, and hope". I believe it is currently at its fortieth or
so QA release...
Paul Keinanen
wrote:
>Reminds me of some C code on the internet that I saw in which someone
>decided to use a base 2 log to determine if a bit in a variable was
>set!! I mean, why do these complicated things if there is a much
>easier, simpler (and MUCH faster) way to accomplish it?
Most likely the person had previously only been working with languages
that did not support bitwise operations.
When people with wildly different backgrounds work on a project, the
project manager should follow the new persons quite closely for a few
months to get a way with their previously learned "bad habits".
I have been working with realtime control systems for quite a while, I
noticed that it is very hard to learn people with batch processing
background to understand the concepts of multitasking. Persons that
have only worked with 8 bitters are very eager to sneak some busy
loops into the code when they think there is nothing useful to do for
a while and Windows programmers tend use a huge amount of resources
indiscriminately :-).
When getting new people with very different background into a new
project, quite a lot of resources should be allocated for
familiarisation to the new environment in the beginning of the
project.
The worst thing to do when the deadlines are approaching very fast, is
to add new persons with wildly different experience into the project
("he/she can program and thus help the project finish in time"), since
the current project staff would have to supervise the newcomers that
will not be very productive in a month or two.
Paul
Vivekanandan M
I also worked on interfacing a LCD and a keyboard to 8051 before 6
years! It was an interesting experience. You should be very careful
while initialising the LCD, I mean the timing signal generated by your
8051 using for loop. Btw, which LCD you are using. Do you have any idea
about the hardware level interfacing - how LCD is connected to 8051,
through multiplexers, buffers, etc...
Best Regards,
Vivekanandan M
Hans-Bernhard Broeker
> int func(int param)
> {
> if (param)
> do_something((int) do_something_else);
Strike 1: Casting a function pointer to an integer.
> else if (param)
> do_something_else();
> }
> int do_something (int param) {
> if (param == 1)
Strike 2: comparing a parameter that may well be a casted function
pointer to an integer literal. That's completely out of line.
> do_something_else();
> label:
> asm {
> mov a,#1
> call do_something_else
> jmp do_something
> }
> }
> int do_something_else(void) {
> if (global_variable) goto label;
Strike 3: goto from one function to another. In an anywhere nearly
sane toolchain, this shouldn't even compile.
That's three counts of totally inadmissable C code, before we even
start looking at violent abuse of inline assembly. Let's just say
that if that's not immediate grounds for firing whoever wrote this,
and everybody else who let it slip past after having actually seen it,
and closely re-evaluating everybody ever hired by any of them, I find
it hard to imagine what might be.
> else asm {
> ret;
> }
> }
The only way to understand what this nightmare might do probably is to
take a full day looking at the assembly code, after having taken a
very good look into the platforms's C ABI. Then rewrite it from
scratch either completely in C, or completely in ASM.
I strongly suggest you convince management to set a prize of a couple
thousand dollars' worth of stocks in the company for fixing this
horror.
--
Hans-Bernhard Broeker (bro...@physik.rwth-aachen.de)
Even if all the snow were burnt, ashes would remain.
CBFalconer
> larwe wrote:
>
... snip ...
>
> Also Akshay, you can basically take the examples Iarwe posted as
> pitfalls to avoid when coding for an embedded system. Not only
> will you save yourself headaches and stress, you will have a lot
> more fun doing your project :).
He isn't bothering to point it out, but Lewis is blessed with an
abundance of initials, none of which are 'I'.
--
Some informative links:
news:news.announce.newusers
http://www.geocities.com/nnqweb/
http://www.catb.org/~esr/faqs/smart-questions.html
http://www.caliburn.nl/topposting.html
http://www.netmeister.org/news/learn2quote.html
larwe
CBFalconer wrote:
> He isn't bothering to point it out, but Lewis is blessed with an
> abundance of initials, none of which are 'I'.
I wasn't going to bother to point this out either, but I am likewise
blessed with an abundance of Christian names, none of which are
'Lewis'.
-- Lewin A.R.W. Edwards
Crusher Royal by appointment to HRH Elizabeth R II.
Available for George Formby impressions. Own ukelele.
larwe
Hans-Bernhard Broeker wrote:
> Strike 1: Casting a function pointer to an integer.
> Strike 2: comparing a parameter that may well be a casted function
> Strike 3: goto from one function to another. In an anywhere nearly
You missed - but it probably wasn't obvious - the fact that it goes
into a C function (which happens to have a stack frame) and returns
with a simple ret buried in some inline assembler. It amazes me that
this code worked at all. Taking a quick peek in the debugger (through
dark glasses to avoid eye damage) shows that the top word on the stack
is a value that HAPPENS to point to some innocuous code that is
followed by a ret. So the stack gets unwound "correctly" even though
the program counter has to travel through a black hole in the process.
> that if that's not immediate grounds for firing whoever wrote this,
> and everybody else who let it slip past after having actually seen it,
> and closely re-evaluating everybody ever hired by any of them, I find
This code originates from an enclave - some might say "ghetto" - in the
company that no longer exists. The staff from that enclave - good, bad
and ugly - have been distributed throughout the department... we scored
both this project and the person who wrote this code. Ladies and
gentlemen, step right up - three shies for a penny, every child wins a
prize!
> The only way to understand what this nightmare might do probably is to
> take a full day looking at the assembly code, after having taken a
Well, when this plush assignment came my way, I looked at it very
briefly, decided that this had "delegate" written all over it, and
promptly delivered it onto someone else's desk. I get status updates
every few days, detailing the buried treasure uncovered thus far.
> I strongly suggest you convince management to set a prize of a couple
> thousand dollars' worth of stocks in the company for fixing this
It needs rewriting almost entirely from scratch, and it's a goal of
mine to do the rewrite. However first it is necessary (IMHO :)) to
generate a formal specification for the project; I'm working on that.
Isaac Bosompem
CBFalconer wrote:
..snip..
> He isn't bothering to point it out, but Lewis is blessed with an
> abundance of initials, none of which are 'I'.
>
Sorry Larwe (sp?) for bastardizing your name, it was unintentional.
I know this will get on your nerves CB, but I am using Google Groups to
post here. I have used Mozilla's client which I sorely miss, but my ISP
dropped USENET service a few months ago. No reason was given as to why?
The fonts they use on this thing made it difficult to differentiate
between a 'I' and 'L'.
CBFalconer
> CBFalconer wrote:
>
>> He isn't bothering to point it out, but Lewis is blessed with an
>> abundance of initials, none of which are 'I'.
>
> I wasn't going to bother to point this out either, but I am
> likewise blessed with an abundance of Christian names, none of
> which are 'Lewis'.
Sorry 'bout that. I won't try to put it down to a typo. s and n
are too far apart.
Bob
<cbfal...@yahoo.com> writes
>larwe wrote:
>> CBFalconer wrote:
>>
>>> He isn't bothering to point it out, but Lewis is blessed with an
>>> abundance of initials, none of which are 'I'.
>>
>> I wasn't going to bother to point this out either, but I am
>> likewise blessed with an abundance of Christian names, none of
>> which are 'Lewis'.
>
>Sorry 'bout that. I won't try to put it down to a typo. s and n
>are too far apart.
Maybe your SN ratio is incorrect...
Steve at fivetrees
news:1143428515.3...@j33g2000cwa.googlegroups.com...
> The genral consensus of C pro's though is to avoid using goto. To tell
> you the truth, easier said than done in some specific cases.
I disagree. I haven't used a GOTO in C or any other language (other than
BASIC, which doesn't count!) in about 25 years.
And (in response to another poster), no, avoiding GOTOs needn't result in
hard-to-read code. It just makes you think a bit harder about why the bit
you're breaking from is so deeply embedded in the first place, and why
perhaps e.g. a state machine might be a better mousetrap.
larwe
Akshay wrote:
> I posted here only to get help, not to get flamed.
You weren't getting flamed (much). We're merely in a discussion of the
[de]merits of the goto keyword in C, and I was sharing a bunch of
experiences on a piece of code (not yours) in which "goto" barely
registers on the sin scale vs. the horrific errors and hairiness.
I can proudly(?) say I've never typed "goto" into a C source file. I
have, however, maintained code using goto and not removed it, which I
guess is much the same as the difference between murder and
manslaughter.
Steve at fivetrees
news:1143479846.5...@z34g2000cwc.googlegroups.com...
>
> I can proudly(?) say I've never typed "goto" into a C source file. I
> have, however, maintained code using goto and not removed it, which I
> guess is much the same as the difference between murder and
> manslaughter.
;)
Fairly recently I had to deal with some code written by a company's "Linux"
expert. His main() function had several thousand lines and 30-odd GOTOs.
Sigh.
Hans-Bernhard Broeker
> Hans-Bernhard Broeker wrote:
> > Strike 1: Casting a function pointer to an integer.
> > Strike 2: comparing a parameter that may well be a casted function
> > Strike 3: goto from one function to another. In an anywhere nearly
> You missed - but it probably wasn't obvious - the fact that it goes
> into a C function (which happens to have a stack frame) and returns
> with a simple ret buried in some inline assembler.
I didn't exactly miss it. First off all, three strikes is Out, so no
need to look any further. Second, I did say I was not even looking at
the inline asm part of that work of ... well, let's be generous and
call it "art".
Outside the IOCCC and possibly some *very* serious case of attempted
security by obscurity (say, a copy protection scheme for a
millions-of-dollars product), there cannot possibly be any reason for
such code to exist.
Tauno Voipio
This is a symptom of trying to solve the problems of
colonels, lietenants and sergeants in a single function,
instead of splitting them to separate functions.
Back to drawing board ...
--
Tauno Voipio
tauno voipio (at) iki fi
larwe
Hans-Bernhard Broeker wrote:
> I didn't exactly miss it. First off all, three strikes is Out, so no
Maybe so, but some people deserve to receive continued beating even
when they are down on the ground unconscious.
> the inline asm part of that work of ... well, let's be generous and
> call it "art".
:)))))
> Outside the IOCCC and possibly some *very* serious case of attempted
> security by obscurity (say, a copy protection scheme for a
> millions-of-dollars product), there cannot possibly be any reason for
> such code to exist.
"Never ascribe to malice that which can adequately be explained by
incompetence".
It's surprisingly difficult to get rid of people. This person has
_potential_ but needs severe flogging in order to realize it.
Steve at fivetrees
news:YDWVf.210$N84...@read3.inet.fi...
> Steve at fivetrees wrote:
>> Fairly recently I had to deal with some code written by a company's
>> "Linux" expert. His main() function had several thousand lines and 30-odd
>> GOTOs. Sigh.
>
> This is a symptom of trying to solve the problems of
> colonels, lietenants and sergeants in a single function,
> instead of splitting them to separate functions.
>
> Back to drawing board ...
I'd say it's a symptom of being clueless ;).
There was much duplication - same code with different magic numbers
(aaaargh!). I would have suggested factoring, but became convinced it was a
lost cause, and wrote my own code instead...
Hans-Bernhard Broeker
> It's surprisingly difficult to get rid of people. This person has
> _potential_ but needs severe flogging in order to realize it.
I take it you've already put out the order for a sturdy LART
(L??er Attitude Readjustment Tool, a.k.a. clue-by-four) to be
shipped to your office, then?
Steve at fivetrees
> "Never ascribe to malice that which can adequately be explained by
> incompetence".
>
> It's surprisingly difficult to get rid of people. This person has
> _potential_ but needs severe flogging in order to realize it.
It's kinda scary. I deal/have dealt with any amount of coders, but have only
once or twice run into people who write "good" code - not just by my
standards, but by *any* standards. I've worked in R&D departments where my
style of coding (defensive, structured, OO/highly-modular, and as clear as
possible) has been referred to as "fastidious" (their term: I prefer
"thorough"). Yet the fact that I get the project finished sooner and with
fewer (if any) bugs seems lost on them. I would have thought that would be
the one true metric, but hey...
Actually, I'm being slightly unfair. One tech director *did* notice this,
and invited me back to indoctrinate the R&D dept and to co-write [1] a
coding standards proposal. Very brave of him, I thought. I failed miserably.
I rapidly realised my only hope was to sack the entire team, and start
recruiting from scratch. Which was not an option.
[1] I knew I was in trouble when my co-writer, a senior employee, *insisted*
on testing Boolean variables for equality with TRUE and FALSE. For example:
a) if ( boolean_variable == FALSE )
b) if ( boolean_variable == TRUE )
I did try countering with:
a) if (( boolean_variable == FALSE ) == TRUE )
b) if (((( boolean_variable == TRUE ) == TRUE ) == TRUE ) == TRUE )
To no avail. I also tried to explain that while FALSE == 0 was reasonable,
TRUE == #some_fixed_value was not. I also tried pointing out the many
examples in K&R where non-zero means TRUE. I was told the issue was
non-negotiable. I walked.
Summary: in hardware, schematics are subject to peer review simply because
colleagues have to read the schematics. Not necessarily so with software.
I've become a big fan of non-confrontational code reviews for just this
reason. It gives me an opportunity to show what good code should look like
(and I'm being entirely serious). And I'm always open to ways to improve my
own coding - if there's a valid criticism of my code or my approach, I
genuinely want to know. This seems to be unusual.
larwe
Hans-Bernhard Broeker wrote:
> I take it you've already put out the order for a sturdy LART
> (L??er Attitude Readjustment Tool, a.k.a. clue-by-four) to be
> shipped to your office, then?
I have a tool on my wall called "the Motivator". It consists of about a
foot of [PLCC44] IC tube with about two feet of multicore cable
attached to the end. The cable was the tail of a Microsoft bus mouse, I
think. The last foot of the cable has the outer insulation removed and
the cores inside spread out. It's a cat-o-five-tails.
CBFalconer
> "Tauno Voipio" <tauno....@INVALIDiki.fi> wrote in message
>
>>> Fairly recently I had to deal with some code written by a
>>> company's "Linux" expert. His main() function had several
>>> thousand lines and 30-odd GOTOs. Sigh.
>>
>> This is a symptom of trying to solve the problems of
>> colonels, lietenants and sergeants in a single function,
>> instead of splitting them to separate functions.
>>
>> Back to drawing board ...
>
> I'd say it's a symptom of being clueless ;).
>
> There was much duplication - same code with different magic
> numbers (aaaargh!). I would have suggested factoring, but became
> convinced it was a lost cause, and wrote my own code instead...
The advantage to initial factoring is that you have whatever tests
applied to the original available, or you can even develop some
easily run regression tests. Then you can factor things out one at
a time, with a built in curb on your enthusiasm. That also means
you postpone the temptation to add features until the factoring is
complete.
I have found this especially attractive when the original was
written by someone who understood the problem well, but was totally
clueless about how to program it. You can learn a lot from a
clueless program.
--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>
Spehro Pefhany
<zwsd...@gmail.com> wrote:
An empty tube from 300-mil DIPs is good for swatting a big
high-spirited dog on the flanks. It doesn't have enough mass to hurt
them, but it makes a loud noise, which gets their attention and which
they really don't like.
Best regards,
Spehro Pefhany
--
"it's the network..." "The Journey is the reward"
sp...@interlog.com Info for manufacturers: http://www.trexon.com
Embedded software/hardware/analog Info for designers: http://www.speff.com
EggNChips
> On 26 Mar 2006 21:37:30 GMT, "John B"
> <spamj_ba...@blockerzetnet.co.uk> wrote:
>
>> If he writes C code with "goto" in it he's not capable of learning
>> anything anyway.
>
> IMHO, the use of goto is quite appropriate in this case, since C only
> supports exiting the innermost loop with break. A selective use of
> goto can often improve code readability and reduces the level of
> indent required.
>
> Some seems to fear the "horrible" word goto so much that they rather
> use complex flags to control loop execution etc. making programs very
> hard to read.
>
> Paul
>
I agree, goto can save loads of memory and improve readability of code.
I also agree however, that indiscriminate use of goto can drastically
reduce the readability of code.
If goto was that bad then it would have been removed from the Ansi standard.
Steve at fivetrees
news:SAaWf.285672$YJ4....@fe2.news.blueyonder.co.uk...
>>
>> Some seems to fear the "horrible" word goto so much that they rather
>> use complex flags to control loop execution etc. making programs very
>> hard to read.
>>
>
> also agree however, that indiscriminate use of goto can drastically reduce
> the readability of code.
This is quite a common myth. The only time I've found it to be true has been
when the code was poorly structured in the first place.
EggNChips
tasks before a function returns. If the function is needs to return
because of a fail condition. Yes there are other ways of doing it, but
a goto is easier to read and saves duplicating code.
void Fuction()
{
if (!CarryOutSomeTask()) goto ExitFunction;
do;
some;
actions;
ExitFuction:
do;
some;
cleanup;
that needs done;
regardless;
of success;
}
Kelly Hall
> I have a tool on my wall called "the Motivator". It consists of about a
> foot of [PLCC44] IC tube with about two feet of multicore cable
> attached to the end. The cable was the tail of a Microsoft bus mouse, I
> think. The last foot of the cable has the outer insulation removed and
> the cores inside spread out. It's a cat-o-five-tails.
We have something similar at work that we call "the Productivator".
Kelly
John B
That is complete bull...t. It's very difficult to spot the '!' in the
conditional. A typical example of negative thinking.
void Fuction()
{
if (CarryOutSomeTask())
{
do;
some;
actions;
}
do;
some;
cleanup;
that needs done;
regardless;
of success;
}
would be the correct way to write that code.
--
John B
Steve at fivetrees
My solution(s) would be (IMHO) easier to read, easier to expand, and would
have no duplication.
1) Break it down vertically. Maintain a flag that means "cleanup required".
Skip any other actions if flag is set. Equivalent code to yours (with no
need for a flag in this instance) would be:
void Function()
{
if ( CarryOutSomeTask() )
{
actions;
}
cleanup_actions;
}
2) Use a state machine, and put it in a loop. A fairly classic case would be
an input stream parser; far too many times I've seen errors handled with
GOTOs. Using a state machine allows all the error handling to be put in one
place. Changing state to the error state is logically equivalent to a GOTO,
but far cleaner, and far more expandable. Rather than thinking in terms of
sequences with exception, think vertically.
3) Etc etc.
There are *always* better ways of coding than using GOTOs. These other ways
are always more expandable, more readable, have fewer side effects, and
reflect more directly the problem you're addressing.
FWIW: I've done a great deal of legacy-code cleanups. I've never had any
trouble eliminating all GOTOs, and I've often removed latent bugs and
side-effects in the process. I've been doing this stuff long enough to
remember when "structured programming" was considered a fad; I think those
days are long gone. At first I ran into cases where I *thought* I needed a
GOTO; with experience I realised I just hadn't thought the problem through
far enough. Nowadays I recognise such cases as meaning "restructure".
GOTOs may be legal, but they promote bad habits and wooly thinking. Avoiding
them makes you think more in terms of modularity and generality. This is A
Good Thing. And no less a luminary than Dijkstra had something to say on the
subject:
http://www.acm.org/classics/oct95/
Anton Erasmus
I have had cases where the only way to get the compiler to generate
optimal code for a simple memcpy type loop was to use a goto. Using
any other looping structure generated very non optimal code. Other
(better) compilers generated the same code from a while loop as from
the code using gotos. I do agree that if one uses gotos to break out
of very convoluted code, then there is probably something wrong.
Another use for gotos is writing fast byte interpreters on gcc, where
calculated gotos can help a lot.
Regards
Anton Erasmus
Steve at fivetrees
news:tl6j22pmn53c79789...@4ax.com...
>
> I have had cases where the only way to get the compiler to generate
> optimal code for a simple memcpy type loop was to use a goto. Using
> any other looping structure generated very non optimal code. Other
> (better) compilers generated the same code from a while loop as from
> the code using gotos.
Eep! Sounds like a compiler issue. Your solution falls under the category of
"compiler-specific optimisation", which I'd argue was an entirely different
issue from "good design".
> I do agree that if one uses gotos to break out
> of very convoluted code, then there is probably something wrong.
> Another use for gotos is writing fast byte interpreters on gcc, where
> calculated gotos can help a lot.
Again, probably compiler-specific. I have no big problem with this: I've
used jump tables in assembler, and I routinely use function tables in C.
FWIW, I'm more defending pure structured design than I am promoting pure C.
I used to design in structured English, and code in assembler. Nowadays I
use C for both purposes, but I'm still very cautious about separating design
from code, if you see what I mean.
CBFalconer
> EggNChips scrobe on the papyrus:
>
Which is even more descriptive as:
void Fuction()
{
if (CarryOutSomeTask()) {
do; some; actions;
}
else {
cleanup; after; failure;
}
do; some; cleanup; that needs done; regardless; of success;
}
Yet I am an advocate of goto in the appropriate places, which are
fairly rare.
--
fix (vb.): 1. to paper over, obscure, hide from public view; 2.
to work around, in a way that produces unintended consequences
that are worse than the original problem. Usage: "Windows ME
fixes many of the shortcomings of Windows 98 SE". - Hutchison
Dave Hansen
<spamj_ba...@blockerzetnet.co.uk> wrote:
>EggNChips scrobe on the papyrus:
>
[... re: goto ...]
These are both poor examples.
I don't recall ever writing a goto in embedded code, but there was a
time when I was implementing desktop APIs (to control embedded
systems). The APIs generally took the classic form of files: open the
device, perform some operations, close the device when finished.
The open functions tended to be somewhat complex. Ideally, they would
be a matter of allocating the required data structures, filling them
in, and returning a "handle" to the descriptor which the user would
pass in to the operating functions to identify the device. Oftentimes
the required data structures were complex, and depended on
configuration information in external files or the availability of
attached hardware. And of course, memory allocation can fail at any
time.
So you wind up with something like
Device_Handle *Device_Open(<parameters>)
{
Device_Handle *p = NULL;
if (<parameters OK>)
{
p = malloc(sizeof(Device_Handle));
if (p)
{
p->some_substructure = create_sub();
if (p->some_substructure)
{
p->etc = create_etc();
if (!p->etc)
{
cleanup_substructure(p->some_substructure();
cleanup_Handle(p);
p = NULL
}
}
else
{
cleanup_Handle(p);
p = NULL;
}
}
}
return p;
}
or (sometimes cleaner and easier to understand):
Device_Handle *Device_Open(<parameters>)
{
Device_Handle *p = NULL;
if (<parameters Not OK>)
goto error_exit;
p = malloc(sizeof(Device_Handle));
if (!p)
goto error_exit;
p->some_substructure = create_sub();
if (!p->some_substructure)
goto error_exit;
p->etc = create_etc();
if (!p->etc)
goto error_exit;
return p;
error_exit:
if (p)
{
if (p->some_substructure)
cleanup_substructure(p->some_substructure();
cleanup_Handle(p);
}
return NULL;
}
The second solution reduces the amount of duplicate code and gives the
code a more linear structure that is easier to understand. IMHO,
anyway. The difference is greater the more substructures you have to
create and/or initialize.
Regards,
-=Dave
--
Change is inevitable, progress is not.
Steve at fivetrees
news:76fj22hm0n64q8d3g...@4ax.com...
>
> Device_Handle *Device_Open(<parameters>)
> {
> Device_Handle *p = NULL;
>
> if (<parameters Not OK>)
> goto error_exit;
>
> p = malloc(sizeof(Device_Handle));
> if (!p)
> goto error_exit;
>
> p->some_substructure = create_sub();
> if (!p->some_substructure)
> goto error_exit;
>
> p->etc = create_etc();
> if (!p->etc)
> goto error_exit;
>
> return p;
>
> error_exit:
> if (p)
> {
> if (p->some_substructure)
> cleanup_substructure(p->some_substructure();
>
> cleanup_Handle(p);
> }
> return NULL;
> }
Heh. A C++ coder would say this is an ideal candidate for a throw/exception.
In C, I'd he happy with your code - so long as the error_exit code
explicitly avoids any assumptions (as yours does - although I think I'd add
cleanup for p->etc as a defence against future expansion, even though it's
not needed in your example).
My version (left entirely to my own devices, and I'll explain my aims
shortly) would be:
Device_Handle *Device_Open( <parameters> )
{
Device_Handle *p;
Boolean ok = TRUE;
if ( <parameters OK> )
return( NULL );
p = malloc( sizeof( Device_Handle ));
if ( p == NULL )
return( NULL );
// p known to exist from here on in: make sure we stay in a consistent
state:
p->some_substructure = NULL;
p->etc = NULL;
if ( ok ) // defensive
{
if ( ! ( p->some_substructure = create_sub() ))
ok = FALSE;
}
if ( ok )
{
if ( ! ( p->etc = create_etc() ))
ok = FALSE;
}
// Cleanup any errors arising:
if ( ! ok )
{
if ( p->some_substructure != NULL )
cleanup_substructure( p->some_substructure );
if ( p->etc != NULL )
cleanup_etc( p->etc );
cleanup_Handle( p );
}
return(( ok ) ? p : NULL );
}
My aim throughout is to remove future traps (those assumptions that, while
valid at the time, come back and bite you later when the code gets
expanded). I'm trying to decouple what's happening now from what I have to
do later. Yes, I'm having to do some unwinding, but it's all in one place -
no duplication (ok, there's the returns and the flag setting/testing... but
they're cheap, explicit, and defensive). I'm also creating a template to
make it easily expandable - I'm being very explicit, and providing obvious
slots to add things later.
I should probably add that my "style" comes from having had to maintain
codebases (alone or in teams) over long periods (in some cases, a couple of
decades). I'm acutely aware that I (or some other sucker) will come back to
this, add something, and maybe break it in the process. So I've developed a
defensive style that tries to anticipate the traps I may be laying for
myself.
Disclaimer: it's late, and I'm tired, so I hope to Dog I've made no logical
errors. I'm *so* gonna get jumped on if I have ;).
Steve at fivetrees
news:94CdnQM3NeIHQLTZ...@pipex.net...
>
> if ( <parameters OK> )
> return( NULL );
Bumsbumsbums. The test should be inverted. I plead insanitary.
Steve at fivetrees
Realised I'd replied badly. The "common myth" referred to your first
sentence (GOTO saving loads of memory and improving readability).
The second part, I agree with strongly. I can live with GOTOs where they are
equivalent to a return or a break - i.e. they don't break structure. Once
one gets into spaghetti code, though, all bets are off. The acid test is
when you don't quite know how you got here - if you don't know your context
(or can't deduce it), you're in deep trouble. I believe this to be
unarguably true.
larwe
Steve at fivetrees wrote:
> > if ( <parameters OK> )
> > return( NULL );
>
> Bumsbumsbums. The test should be inverted. I plead insanitary.
Is that like "I sing the body electric"?
Steve at fivetrees
No. Or possibly yes. Or possibly vice-versa.
See? told you I was tired.
larwe
Steve at fivetrees wrote:
> See? told you I was tired.
Tired? My book ms. is due to the publisher on Friday, and I'm still
working on the edits people sent in. I guesstimate another 10K words to
write based on the notes I'm reading here.
I have a calculus exam on Thursday afternoon for which at least cursory
revision is necessary.
I have a board to bring up by Monday, which is arriving via FedEx
tomorrow. It's the (hopefully) final spin of a commercial product, and
it contains essentially untested changes.
I have a 2500 word article due in about two weeks (part 5 of my "mad
Mac mini" series).
I need to outline the next two articles in my Kuro Box x86-to-PPC
migration series and start work on the next one.
I need to dream up scripts for the first couple of episodes of a
technical Internet radio broadcast, and find enough of my studio
equipment to record at least one of them. Deadline, 5/5.
This is not even counting the people who have contacted me in the past
four weeks asking if I could quote them to work on a project... some of
them got really insistent when I said I'm >125% loaded at this time.
One company sent me a half-case of wine (not bad stuff) with a request
to consider scheduling in their project when possible. Not sure how
wine is supposed to help with that ;)
I guess this qualifies as a busy week. By Monday I'll either be
completely caught up with the immediate requirements, or I'll have a
heart attack.
Tauno Voipio
(-- clip clip --)
>
>
> I have had cases where the only way to get the compiler to generate
> optimal code for a simple memcpy type loop was to use a goto. Using
> any other looping structure generated very non optimal code. Other
> (better) compilers generated the same code from a while loop as from
> the code using gotos. I do agree that if one uses gotos to break out
> of very convoluted code, then there is probably something wrong.
> Another use for gotos is writing fast byte interpreters on gcc, where
> calculated gotos can help a lot.
>
> Regards
> Anton Erasmus
>
>
Here's an example with a modern C compiler (GCC 3.4.3, ARM):
/* copy.c - memcpy() loop in C */
/* Tauno Voipio */
void copy(const unsigned char *src, unsigned char *dst, unsigned int count)
{
do
*dst++ = *src++;
while (--count != 0);
}
ARM GAS /var/tmp//cc6fhgqr.s page 1
1 .file "copy.c"
2 .text
3 .align 2
4 .global copy
5 .type copy, %function
6 copy:
10 @ lr needed for prologue
11 .L2:
12 0000 0130D0E4 ldrb r3, [r0], #1
13 0004 012052E2 subs r2, r2, #1
14 0008 0130C1E4 strb r3, [r1], #1
15 000c FEFFFF1A bne .L2
16 0010 0EF0A0E1 mov pc, lr
17 .size copy, .-copy
18 .ident "GCC: (GNU) 3.4.3"
----
I just wonder what an assembly language programmer could
do better here - with or without a goto.
There's one point, however. This version does not behave
well if count is zero. The change costs an if statement
encompassing the loop:
if (count != 0)
{
/* the loop from above */
}
The cost in assembly code is two words:
10 0000 000052E3 cmp r2, #0
12 0004 0EF0A001 moveq pc, lr
Again, here's littele what assembly programmer or goto could do.
EggNChips
That's a bit strong isn't it? The example was purely describing the use
of goto and showing that it doesn't have to obfuscate the code. I would
guess that over 50% of professional programmers use that shorthand?????
Writing code is like writing a letter, there is no correct way to write
it and everybody has their own styles, as long as it is easily readable,
functional and efficient.
EggNChips
thing. As I have said in another post, writing code is like writing a
letter, everyone has their own style and as long as it is easy to read,
functional and efficient, then write it how you like.
IMO goto's aren't the incarnate of evil and if the programmer feels
appropriate they can be used effectively. I generally never use more
than one in a function, I have also seen code with excessive goto's and
it's not pleasant.
John B
The point I was making was just that. Because of the 'goto' the rest of
the code was obfuscated and too complex.
>
> Writing code is like writing a letter, there is no correct way to
> write it and everybody has their own styles, as long as it is easily
> readable, functional and efficient.
Unfortunately, your example was none of those.
--
John B
EggNChips
>
> The point I was making was just that. Because of the 'goto' the rest of
> the code was obfuscated and too complex.
>
If you think that's obfuscated and complex then you suck as a programmer :-)
>> Writing code is like writing a letter, there is no correct way to
>> write it and everybody has their own styles, as long as it is easily
>> readable, functional and efficient.
>
> Unfortunately, your example was none of those.
see above :-)
John B
.
.
>
> If you think that's obfuscated and complex then you suck as a
> programmer :-)
.
.
Suck, blow ... it's all the same to me. I've been writing 'C' since
1979 and I bet you weren't even born then.
--
John B
Paul Keinanen
<spamj_ba...@blockerzetnet.co.uk> wrote:
>EggNChips scrobe on the papyrus:
>> If you think that's obfuscated and complex then you suck as a
>> programmer :-)
>Suck, blow ... it's all the same to me. I've been writing 'C' since
>1979 and I bet you weren't even born then.
In those days both C and Pascal were just toy languages :-).
OK, I rewrote a Pascal run time library for a specific platform (with
a hardware floating point processor) in the late 1970's just to show
that such languages could be used for some serious scientific work.
Before that, Fortran IV was used, with only the DO-loop and the GOTO
statement as the only control structures. Even with Fortran IV, you
can write quite readable code by using GOTOs only for forward jumps
and use the DO loops for repetitions (even if that might sound a bit
artificial).
Using C/Pascal style structured languages greatly reduces the need for
gotos, but they do not eliminate it completely. I use it 0-3 times in
each source file without being ashamed.
There seems to be horror stories about Basic destroying your brain or
about GOTOs creating huge spaghetti nests, but the only real spaghetti
code nests I have seen in real production code were related to Cobol
PERFORM statements, in which restructuring might have helped.
Paul
Frank Bemelman
news:4429A1AC...@yahoo.com...
>
> Yet I am an advocate of goto in the appropriate places, which are
> fairly rare.
So am I. For backward jumps as well. Sometimes it is just
too much hassle to wrap everything in if/then/elses. Fairly
rare, yes, like 'continue' which I use even less but probably
should consider to use more often.
Use what is available and practical.
--
Thanks, Frank.
(remove 'q' and '.invalid' when replying by email)
CBFalconer
>> fairly rare.
>
> So am I. For backward jumps as well. Sometimes it is just
> too much hassle to wrap everything in if/then/elses. Fairly
> rare, yes, like 'continue' which I use even less but probably
> should consider to use more often.
>
> Use what is available and practical.
My principle use of continue is as a marker for empty statements,
as in:
while ((EOF != (ch =getchar())) && (ch != '\n')) continue;
It stands out much better than a nude semicolon.
--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>