[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