Building a query string based on variable values. Where is the mistake?

Question

I am trying to create a query string based on GET values passed to common vars:

if isset, gTipo = $_GET['tipo'] and others like this.

So, here is the code that is not working:

$sqlLista   =   'SELECT * FROM produtos';


if($gTipo <> 0 || $gLinha <> 0)
{
    if($gtipo <> 0 && $gLinha == 0 )
    {
        $sqlLista .= ' WHERE id_tipo = '.$gTipo.'';
    }
    if($gtipo <> 0 && $gLinha <> 0)
    {
        $sqlLista .= ' WHERE id_tipo = '.$gTipo.' AND id_linha = '.$gLinha.'';
    }
    if($gTipo == 0 && $gLinha <> 0)
    {
        $sqlLista .= ' WHERE id_linha = '.$gLinha.'';
    }
}

If i set my url as ?tipo=2&linha=4 , my script capture this GET vars and create the common var gTipo and gLinha. If any of this GET are not set, the gTipo or gLinha receive the '0' (zero) value.

When I run the script of query building, nothing is concatened to $sqlLista, except what is done outside the if ( $sqlLista = 'SELECT * FROM produtos'; ).

I am sure this must be a stupid thing that I cannot see. Please, help me =)

Please tell me that you've already sanitized the $gLinha and $gtipo variables against SQL injection.. — damianb
– damianb, Commented Sep 25, 2011 at 15:47
Depends on how you're running queries. Look at mysql_real_escape_string() if you are using the mysql functions, look at PDO prepared statements if you're using PDO. Also, if you're just expecting one of the variables to be integers, you can just typecast them to integer with $variable = (int) $variable; so that they can't be anything BUT an integer (which is recommended for integers, as it is much less processing). — damianb
– damianb, Commented Sep 25, 2011 at 15:50
tks man, I will use $variable = (int) $variable :) I will use just ids integers in this query. — Acchile Biagioli
– Acchile Biagioli, Commented Sep 25, 2011 at 15:52
Look: $gTipo = (int)$_GET['tipo'] or die ('The value is not a integer.'); — Acchile Biagioli
– Acchile Biagioli, Commented Sep 25, 2011 at 15:54
no need to die(). It will just force it to be 0 if it isn't a numeric value. — damianb
– damianb, Commented Sep 25, 2011 at 15:56

Gus · Accepted Answer · 2011-09-25 15:51:26Z

1

I think your problem is variable case:

if($gtipo <> 0...

should be

if($gTipo <> 0...

in 2 places.

answered Sep 25, 2011 at 15:51

Gus

7,3792 gold badges29 silver badges23 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

user7675 Over a year ago

True, variables are case-sensitive. php -r '$Foo = 1; var_dump($foo);'

Acchile Biagioli Over a year ago

Gus, you are absolutelly right!!! I knew that should be a stupid detail that I wasnt seeing. Really tks man. Many hours over this keyboard are blurring my eyes and thoughts XD

Your Common Sense · Accepted Answer · 2011-09-25 16:05:00Z

1

dunno what's your problem but the code seems a bit bloated to me
I'd make it this way

$w = array();
$where = '';
if (!empty($_GET['tipo'])) $w[] = 'id_tipo = '.intval($_GET['tipo']);
if (!empty($_GET['linha'])) $w[] = 'id_linha = '.intval($_GET['linha']);
if ($w) $where = " WHERE ".implode(' AND ',$w);
$sqlLista = SELECT * FROM produtos'.$where;

hope you know what you're doing and you really need AND, not OR in the query.

if you have your validations already, the code would be even shorter

$w = array();
$where = '';
if ($gtipo) $w[] = "id_tipo = $gtipo";
if (gLlinha) $w[] = "id_linha = gLinha";
if ($w) $where = " WHERE ".implode(' AND ',$w);
$sqlLista = 'SELECT * FROM produtos'.$where;

edited Sep 25, 2011 at 16:05

answered Sep 25, 2011 at 15:58

Your Common Sense

158k42 gold badges226 silver badges373 bronze badges

2 Comments

Acchile Biagioli Over a year ago

Tks for help Col. Shrapnel... but I will keep my original code structure. Simple and works.

Your Common Sense Over a year ago

I wouldn't call a bunch of useless operators "simple". your first condition is totally useless

dar7yl · Accepted Answer · 2011-09-25 20:31:30Z

0

Your variable names are inconsistent - $gTipo vs $gtipo.

This line will prevent your inner logic from executing. remove it.

if($gTipo <> 0 || $gLinha <> 0)

As a matter of style, you should use "else if" to prevent multiple "WHERE" lines from being appended if you make a logic error.

answered Sep 25, 2011 at 20:31

dar7yl

3,78528 silver badges20 bronze badges

Collectives™ on Stack Overflow

Building a query string based on variable values. Where is the mistake?

3 Answers 3

2 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related