0

i have one function which calculate total number of records in table and get two arguments and both are optional.

function getTotal($id=0,$id1=0)
    {
        ($id==0?$addQuery="":$addQuery=" where art_id=".$id);
        if($id1<>0 && $id==0)
        {
        $addQuery=" where up_type=".$id1
        }
        if($id1<>0 && $id<>0)
        {
        $addQuery=" and up_type=".$id1
        }
        mysql_set_charset('utf8');
        $query="SELECT COUNT(id) FROM tbl_up ".$addQuery;
        $result=$this->query($query,1);
        return $result;
    }

if you see i write if id is passed then i put the where class in one line

but if 2nd argument id1 is passed or not i need to add text to where class, but here is if id is passed then it should start from and and if id is not passed it should start with where

i try to write if but these lines are too much, i need some thing like first line

($id==0?$addQuery="":$addQuery=" where art_id=".$id);

for 2nd agrument.

Thanks

air
  • 6,136
  • 26
  • 93
  • 125
  • this looks like an SQL-injection hole, see: http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain – Johan Aug 30 '11 at 14:54

3 Answers3

4
function getTotal($id=0,$id1=0)
    {
        $where = array();
        if ($id) $where[]='`art_id`="'.$id.'"';
        if ($id1) $where[] = '`up_type`="'.$id1.'"';
        $where = (!count($where) ? '' : 'WHERE '.implode(' AND ', $where));
            $query="SELECT COUNT(id) FROM tbl_up ".$where;
            mysql_set_charset('utf8');
            $result=$this->query($query,1);
            return $result;
    }
ZigZag
  • 539
  • 1
  • 8
  • 19
2

try:

function getTotal($id = 0,$id1 = 0) {
  // sorry, I rewrite the first expression to this, easier to read IMHO
  $addQuery = $id == 0 ? "" : " where art_id='".mysql_real_escape_string($id)."'";
  if ($id1 <> 0)
    $addQuery .= ($id == 0 ? " where" : " and") . " up_type='".mysql_real_escape_string($id1)."'";
  mysql_set_charset('utf8');
  $query="SELECT COUNT(id) FROM tbl_up ".$addQuery;
  $result=$this->query($query,1);
  return $result;
}
LeleDumbo
  • 9,192
  • 4
  • 24
  • 38
  • -1, if you forget to (single) quote the injected $id's id's that contain spaces will not work, also anti-SQL injection measures you take prior to this snipped will fail. – Johan in 0 seconds edit – Johan Aug 30 '11 at 15:39
  • please don't downvote because of something that OP didn't ask. I gave answer to what he wanted to do, period. that injection thing should be another topic. – LeleDumbo Aug 30 '11 at 17:56
  • I downvoted because it would be dangerous to use this code in production. If you edited the post to not be dangerous I'll be more than happy to upvote. – Johan Aug 30 '11 at 18:25
0
function getTotal($id=0,$id1=0)
{
    $addQuery="where 1=1"
    if($id <>0)  $addQuery.=" and art_id =".$id
    if($id1<>0)  $addQuery.=" and up_type=".$id1

    mysql_set_charset('utf8');
    $query="SELECT COUNT(id) FROM tbl_up ".$addQuery;
    $result=$this->query($query,1);
    return $result;
}

this is a generic way to have multiple cases tested

Saic Siquot
  • 6,513
  • 5
  • 34
  • 56
  • you're missing the `WHERE` clause – fbstj Aug 30 '11 at 14:52
  • -1, if you forget to (single) quote the injected `$id`'s id's that contain spaces will not work, also anti-SQL injection measures you take prior to this snipped will fail. – Johan Aug 30 '11 at 15:39