Hello piouPiouM, just some remarks about your patch. All in all, I like that you take care of having a clean code. Clean is good, but here are some little points: 1. It looks like your editor is removing the trailing white spaces automatically, it forces me to diff with "ignore white spaces". I am not sure if this is good or not. Maybe I should configure my emacs instance to remove them too. 2. Do you want to always compare "CONST op $var" in the if blocks? If so, let's do it from now on. 3. I really prefer $i++ than ++$i when on a line on its own. 4. Ok for the spaces around the "." to concatenate strings, I am not used to it, but it looks like you really prefer it this way. 5. You are doing a lot of strict comparisons, even on strings. Is it really faster and/or better? 6. You prefer && instead of "and", if we go that way, we need to be consistent and use the || too, no? Anyway, thanks for the work, loïc -- Dr Loïc d'Anterroches Founder Céondo Ltd w: www.ceondo.com | e: loic@ceondo.com t: +44 (0)207 183 0016 | f: +44 (0)207 183 0124 Céondo Ltd Dalton House 60 Windsor Avenue London SW19 2RR / United Kingdom
Le 26 nov. 10 à 20:39, Loic d'Anterroches a écrit : > Hello piouPiouM, > > just some remarks about your patch. All in all, I like that you take > care of having a clean code. Clean is good, but here are some > little points: > > 1. It looks like your editor is removing the trailing white spaces > automatically, it forces me to diff with "ignore white spaces". I > am not > sure if this is good or not. Maybe I should configure my emacs > instance > to remove them too. Hello Loïc, The fault is not my editor, it's me: I display invisible characters and I likes see a good eol :) > 2. Do you want to always compare "CONST op $var" in the if blocks? If > so, let's do it from now on. Yes because it's more clean and also it's more faster. > 3. I really prefer $i++ than ++$i when on a line on its own. Ok, I understand (even if the ++$i version is faster than $i++). > 4. Ok for the spaces around the "." to concatenate strings, I am not > used to it, but it looks like you really prefer it this way. A code ventilated is so much more readable ;-) > 5. You are doing a lot of strict comparisons, even on strings. Is it > really faster and/or better? Yes, the binary comparison is faster, even between two operands of same type. > 6. You prefer && instead of "and", if we go that way, we need to be > consistent and use the || too, no? You're right. My preference is for the &&/|| operators because they do not pollute any research into the code on the keywords "and" and "or". > Anyway, thanks for the work, You're welcome. -- Mehdi Kabab Web Developer / Author Code: http://pioupioum.fr My book: http://gimp4you.eu.org/livre/ Twitter: http://twitter.com/piouPiouM
Ok. Everything accepted. I will write down everything as a coding standard "book" so to speak. loïc On 2010-11-27 13:32, Mehdi Kabab wrote: > Le 26 nov. 10 à 20:39, Loic d'Anterroches a écrit : > >> Hello piouPiouM, >> >> just some remarks about your patch. All in all, I like that you take >> care of having a clean code. Clean is good, but here are some >> little points: >> >> 1. It looks like your editor is removing the trailing white spaces >> automatically, it forces me to diff with "ignore white spaces". I >> am not >> sure if this is good or not. Maybe I should configure my emacs >> instance >> to remove them too. > > > Hello Loïc, > > The fault is not my editor, it's me: I display invisible characters > and I likes see a good eol :) > > >> 2. Do you want to always compare "CONST op $var" in the if blocks? If >> so, let's do it from now on. > > Yes because it's more clean and also it's more faster. > > >> 3. I really prefer $i++ than ++$i when on a line on its own. > > Ok, I understand (even if the ++$i version is faster than $i++). > > >> 4. Ok for the spaces around the "." to concatenate strings, I am not >> used to it, but it looks like you really prefer it this way. > > A code ventilated is so much more readable ;-) > > >> 5. You are doing a lot of strict comparisons, even on strings. Is it >> really faster and/or better? > > Yes, the binary comparison is faster, even between two operands of > same type. > > >> 6. You prefer&& instead of "and", if we go that way, we need to be >> consistent and use the || too, no? > > You're right. > My preference is for the&&/|| operators because they do not pollute > any research into the code on the keywords "and" and "or". > > >> Anyway, thanks for the work, > > You're welcome. > > > -- > Mehdi Kabab > Web Developer / Author > > Code: http://pioupioum.fr > My book: http://gimp4you.eu.org/livre/ > Twitter: http://twitter.com/piouPiouM > > -- Dr Loïc d'Anterroches Founder Céondo Ltd w: www.ceondo.com | e: loic@ceondo.com t: +44 (0)207 183 0016 | f: +44 (0)207 183 0124 Céondo Ltd Dalton House 60 Windsor Avenue London SW19 2RR / United Kingdom