Err... how come your I2CStart() and I2CStop() routine
needs to generate a status ?

Unless you are doing multi-master I2C and need to
monitor for bus collision....

... otherwise, isn't it too much to return a status
for the (transmit only) I2C start and stop condition ?

Regards,
Peter Tiang

----- Original Message -----
From: "Michael Rigby-Jones" <mrjones@NORTELNETWORKS.COM>
To: <PICLIST@MITVMA.MIT.EDU>
Sent: Wednesday, April 11, 2001 4:41 PM
Subject: [OT]: Coding style


> This is a plea for some advice on coding style from any of the experts on
> the list.  I have written some I2C drivers for use under Windows for a
> project, and whilst they are perfectly functional I feel that my coding
> style could definately be improved.  Now, a function should preferably
have
> one entry and one exit.  However my code looks something like the function
> below.  Here I am calling several functions that return a boolean to
> indicate success or failure, and I want to exit the calling function if a
> failure is returned.
>
>
> bool _stdcall X9241Write(unsigned char address, unsigned char instruction)
> {
> /*
> Function:               X9241Write
> Arguments:      address = slave address of device, instruction =
instruction
> byte to send to pot
> Return:                 True if success, false if fail.
> Description:    Generic write command for instructions that required no
> additional data.
> */
>
>         unsigned char fullAddr;
>
>     fullAddr = (unsigned char)(X9241_ADDR | (address & 0x0F));
>
>     if(!I2CStart())
>         return false;
>
>     // send slave address
>     if(!I2CTransmit(fullAddr))
>         return false;
>
>     if(!I2CGetAck())
>         return false;
>
>         // send the MSB
>     if(!I2CTransmit(instruction))
>         return false;
>
>     if(!I2CGetAck())
>         return false;
>
>     if(!I2CStop())
>         return false;
>
>     return true;
> }
>
> Now I could nest all the if() statements, but to my mind this would become
> far more confusing than the above, especially with some of the longer
> functions.  Or perhaps set a local variable and check it before calling
each
> sub function i.e.
>
> bool success;
>
> success = I2CStart();
>
> if(success)
>         success = I2CTransmit(fullAddr);
> if(success)
>         success = I2CGetAck();
>
> etc, etc...
>
> This would work and I could use one return at the end, but to my mind a
> failure at the top of the function causes unnecessary overhead in in
> checking the success variable multiple times to reach the end of the
> function. I could use a goto and a label at the end of the function, but
> that seems infinitely worse than multiple returns.
>
> Any other suggestions?
>
> Regards
>
> Mike
>
> --
> http://www.piclist.com hint: To leave the PICList
> mailto:piclist-unsubscribe-request@mitvma.mit.edu
>
>
>

--
http://www.piclist.com hint: To leave the PICList
mailto:piclist-unsubscribe-request@mitvma.mit.edu