NYCPHP Meetup

NYPHP.org

[nycphp-talk] Code review

Leila Lappin damovand at yahoo.com
Mon Jun 6 09:45:57 EDT 2005


Hi,

Thank you for your review.  Interesting to know in how
many ways a code can fail.  I’m going to find a better
way to handle the points you mentioned.

Thanks again for your time and help.


--- Paul Reinheimer <preinheimer at gmail.com> wrote:

> hrm, I got so caught up arguing with the site you
> linked
> that I forgot to look at your code :)
> 
> Four things that stick out during a quick look
> 
> 1. In several places you echo out links to the user
> agent, but encode the
> ampersand, I don't think you need to do this, since
> you are using it as 
> a seperator, not a charecter (or such is my
> understanding).
> eg.
> doQueryIndex.inc
> line 59
>
href="/anp/auth_man.php?event=doQueryLogin&amp;loginAgent=doQueryLogin&amp;target=doQueryIndex
> 
> 2. You're using a global array to store data in
> physiology.php
> $_REQUEST['issue'] =
> htmlspecialchars(urldecode($url_array[4]));
> 
> By storing processed data (in this case by
> urldecode) in a global
> variable, you make impossible for another function
> to determine
> later on wether or not the processing has occured.
> What if I passed
> an issue paramater in either a GET or POST?
> 
> 3. This looks really dangerous
>    $app = $class . '.inc';
>    include ($app);  
> If url fopen wrappers are enabled I can include any
> file I want, from anywhere.
> If you absolutly need to do something like this,
> base it on a switch or if 
> structure, 
> if $class == "something" include "something.inc"
> elseif
> $class == "somethingelse" include
> "somethingelse.inc".
> 
> 4. Your zip structure didn't include any directory
> structure, it's a 
> good idea to keep your .inc files out of the
> document root. This way
> they can't be viewed by end users, or executed
> unexpectedly.
> 
> 
> 
> paul
> 
> 
> 
> On 6/5/05, Leila Lappin <damovand at yahoo.com> wrote:
> > Hi,
> > 
> > I don't know if making my code publicly available
> > would help since my scripts might not be useful to
> > anyone, so they might not get anyone's attention. 
> I
> > try to make my stuff interesting in terms of
> coding
> > style.  I definitely know what I need to do but
> having
> > useful tips how to do certain things is something
> > else.  I'm sending a few of my scripts (in a
> zipped
> > file) anything you care to browse through and
> comment
> > would be great.   I'm using PHP4.3.5 so there
> isn't
> > much object oriented coding. That's too bad since
> I
> > could really do it up with features like protected
> and
> > private members.
> > 
> > Thank you in advance for your time, and in case
> you're
> > interested in a comparison between Java and PHP I
> > found this link that you may like
> > http://www.tek271.com/articles/JavaOrPhp.html and
> find
> > useful.
> > 
> > 
> > --- Christopher Hendry <chendry at gmail.com> wrote:
> > 
> > > Is it code you can make publicly available, I'm
> sure
> > > you'll get plenty
> > > of feedback if you do  :)
> > >
> > > Otherwise, send me some snippets off list, I'd
> love
> > > to see how a
> > > C++/Java programmer is using PHP.
> > >
> > > Is it PHP5?
> > >
> > > C
> > >
> > >
> > > --
> > >
> > > "When you do things right, people won't be sure
> > > you've done anything at all."
> > > _______________________________________________
> > > New York PHP Talk Mailing List
> > > AMP Technology
> > > Supporting Apache, MySQL and PHP
> > > http://lists.nyphp.org/mailman/listinfo/talk
> > > http://www.nyphp.org
> > >
> > 
> > 
> > 
> > __________________________________
> > Yahoo! Mail Mobile
> > Take Yahoo! Mail with you! Check email on your
> mobile phone.
> > http://mobile.yahoo.com/learn/mail
> > 
> > _______________________________________________
> > New York PHP Talk Mailing List
> > AMP Technology
> > Supporting Apache, MySQL and PHP
> > http://lists.nyphp.org/mailman/listinfo/talk
> > http://www.nyphp.org
> > 
> > 
> > 
> 
> 
> -- 
> Paul Reinheimer
> Zend Certified Engineer
> _______________________________________________
> New York PHP Talk Mailing List
> AMP Technology
> Supporting Apache, MySQL and PHP
> http://lists.nyphp.org/mailman/listinfo/talk
> http://www.nyphp.org
> 



		
__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Helps protect you from nasty viruses. 
http://promotions.yahoo.com/new_mail



More information about the talk mailing list