??? 09/08/10 16:27 Read: times |
#178478 - Lots of details to improve on Responding to: ???'s previous message |
1) Use better variable names. "flag_g" isn't a good name. It is a truly lousy name, since you will not be able to tell what it does. It isn't even a boolean flag...
2) No need for: char mystring[] = {"hello world"}; when it's so much nicer with: char mystring[] = "hello world"; 3) Don't use software delays - how long will delay_ms() take if you change compiler, or optimization options? Something wrong with using the processor timer - may even give you a higher grate to show your skills with the timer. 4) Your code can't do anything while waiting for door lock to open or close. Why not run the lock motor in the background and just report a failure if the open/close state doesn't change within allowed time? 5) You do love magic values - why not use more #define or enum { ... } to make the code more readable? if (flag_g==0xaa || flag_g==0xff || flag_g==0xDD) { ... } and same with your keys: if (temp==key_A) { //14 is return/enter, key 15 is escape ... } How silly is it that I need to scan wildly through comments just to figure out that key_A is enter and key_B is ESC? What would have happened if you instead had used key_Enter and key_Esc? 6) Make sure that your comments never lie. You claim: 0xDD is returned if no key is pressed Actual code: else if(x==key_C || x==key_D || x==key_E || x==key_F) { flag=0xDD; lcd_disp1(zero2nine,1); } So in reality, 0xDD is returned if an invalid key is pressed. And your invalid(unsigned char x,unsigned char i) may even return with an undefined result in case x doesn't match any known key. Probably a lot more things to improve, but it takes too long time to study all of the code. |
Topic | Author | Date |
Please assess the c code written by me | 01/01/70 00:00 | |
Lots of details to improve on | 01/01/70 00:00 | |
REALLY confusing | 01/01/70 00:00 | |
timer delay review | 01/01/70 00:00 | |
what about this timer program | 01/01/70 00:00 | |
Still a lot of work to do | 01/01/70 00:00 | |
I have implemented many of the suggestions | 01/01/70 00:00 | |
Still not thinking about your symbol names | 01/01/70 00:00 | |
variable name dilema | 01/01/70 00:00 |