[dev] [PATCH] Adding quota info to IMP summary block
Michael M Slusarz
slusarz at bigworm.colorado.edu
Wed Oct 1 11:37:07 PDT 2003
Quoting Etienne Goyer <etienne.goyer at linuxquebec.com>:
| On Wed, Oct 01, 2003 at 11:14:14AM -0600, Michael M Slusarz wrote:
| > Cleaned up and committed. Thanks.
|
| Thank you very much. Just so I can improve the quality of my patch in
| the future, which part needed cleaning ?
Duplicate/unnecessary code:
* You were checking for the existence of $_SESSION['imp'] although this
should already be taken care of earlier (if $_SESSION['imp'] isn't set, it
would crash long before you got to this block)
* You were calling an if() statement twice where the logic could be done in
a single if() statement
* You could simply append to the $html variable in one case instead of
storing in a temporary variable. There was also no need to predeclare the
$message variable to ''.
Mainly, stupid nit-picky cruft that, after many, many, many, many hours of
staring at PHP/Horde code, you develop a nack for cleaning up :) I'm
probably the only one that cares about this.
michael
______________________________________________
Michael Slusarz [slusarz at bigworm.colorado.edu]
The University of Colorado at Boulder
More information about the dev
mailing list