| View previous topic :: View next topic |
| Author |
Message |
mindstorm88
Joined: 06 Dec 2006
Posts: 102
Location: Montreal , Canada
|
| Code improvement !!!! |
 Posted: Tue Oct 09, 2007 5:59 am |
 |
|
Hi guys this small code block is already working, but as a newbie in C i would like to improve my programming , so how can this code be improve ????
Thanks
What i'm doing is a scan from 0x0011 to 0x008F , but the lowest nibble goes from 1 to F , and the second nibble is 1, 2 ,4 and 8 !!!
data2 start at 0x0011
| Code: |
//===================================
// Scan Lamp driver
//===================================
void scanlamp(void)
{
SPEED250_TIMER_TICKS = UPDN(SPEED250_TIMER_TICKS,1,255);
if(gc_SPEED250_timer)
return;
else
gc_SPEED250_timer = SPEED250_TIMER_TICKS;
data=data2;
Ser_Out(); // sendout serial 16 bits
// here start the part i would like to see improve codewise if possible
if(data2>=0x0081 && data2<0x008f)
data2++;
else if(data2==0x008f)
data2=0x0011;
if(data2>=0x0041 && data2<0x004f)
data2++;
else if(data2==0x004f)
data2=0x0081;
if(data2<0x002f)
data2++;
else if(data2==0x002f)
data2=0x0041;
}
|
Thanks for your time !!
BSL |
|
 |
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Tue Oct 09, 2007 6:41 am |
|
|
Why do you want to improve the code if it is working? I mean, it is always possible to improve any piece of code but if 'good code' does the job nobody is going to pay you extra for 'excellent code'.
I spotted one bug, going from 0x8f your code is going to 0x12, skipping the 0x11 value (you do add twice).
Which leads to the following possible optimization and fix: change all 'if' statements, except the first one, to 'else if'.
Some general remarks:
1) I don't like variables with names like 'data' and 'data2' as these names give me no clue what kind of data is in the variables.
2) You are comparing with values like 0x0081. These are 16 bit values but the maximum value you reach will always fit in an 8 bit integer. It is much more efficient to use 8 bit values and variables consistently.
If I had to implement your code I would have designed it in a way closer resembling the requirements, i.e. in the requirements you are talking about nibbles but this is not directly clear from reading your code. Note that this is a matter of taste, not better. | Code: | ...
data=data2;
Ser_Out(); // sendout serial 16 bits
// here starts the rewritten part
if ( (data2 & 0x0F)==0x0F) // lowest nibble at maximum?
{
if (data2 < 0x80) // Highest nibble not at maximum?
{
data2 = (data2 & 0xF0) * 2; // multiply highest nibble by 2, set low nibble to 0.
data2++; // Skip 0 in lowest nibble
}
else // Highest and lowest nibble both at maximum, restart at 0x11
data2 = 0x11;
}
else
data2++;
|
|
|
|
mindstorm88
Joined: 06 Dec 2006
Posts: 102
Location: Montreal , Canada
|
|
| Posted: Tue Oct 09, 2007 7:40 am |
|
|
Thanks Ckielstra , i do like the way you're doing the IF (that what i meant by improvement)!!!!
The reason i'm using 16 bits is for the Ser_out() function that transmit 16 bits to serial//parrallel circuit , and for that test i'm using only the 8 lsb bits !!
data is long , shifted out by Ser_out()
then if i do
| Code: |
long data;
int data2;
data = 0x0000;
data2 = 0x11;
data = data2;
will data be 0x0011
can i add long with int ?? |
|
|
|
rnielsen
Joined: 23 Sep 2003
Posts: 852
Location: Utah
|
|
| Posted: Tue Oct 09, 2007 8:31 am |
|
|
You need to use apples with apples and can't assume the compiler will always do it properly. You will need to 'case' your variables to ensure they are the same. Try:
| Code: | | data = (int16)data2; |
This will cause the compiler to use data2 as a 16-bit integer during assignment time.
Personally, I like to use the int1, int8, int16 & int32 instead of short, long and so forth. I've not programmed in the 'Windows' world and using int8 makes it easy for me to know what length each variable is.
Ronald |
|
|
adrian
Joined: 08 Sep 2003
Posts: 92
Location: Glasgow, UK
|
|
| Posted: Wed Oct 10, 2007 5:34 am |
|
|
I may be missing something here, but what happens if you enter this code with a value of 0x0F?
| Code: |
// here starts the rewritten part
if ( (data2 & 0x0F)==0x0F) // lowest nibble at maximum?
{
if (data2 < 0x80) // Highest nibble not at maximum?
{
data2 = (data2 & 0xF0) * 2; // multiply highest nibble by 2, set low nibble to 0.
data2++; // Skip 0 in lowest nibble
}
else // Highest and lowest nibble both at maximum, restart at 0x11
data2 = 0x11;
}
else
data2++;
|
With an upper nibble value of 0, 0 * 2 still equals 0. |
|
|
mindstorm88
Joined: 06 Dec 2006
Posts: 102
Location: Montreal , Canada
|
|
| Posted: Wed Oct 10, 2007 6:48 am |
|
|
| You right Adrian !!! but the data always enter this function at 0x0011 (hard coded in main )!!!! |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
