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, Sep 25 '11 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, Sep 25 '11 at 15:50
tks man, I will use $variable = (int) $variable :) I will use just ids integers in this query. — Acchile Biagioli, Sep 25 '11 at 15:52
Look: $gTipo = (int)$_GET['tipo'] or die ('The value is not a integer.'); — Acchile Biagioli, Sep 25 '11 at 15:54
no need to die(). It will just force it to be `0` if it isn't a numeric value. — damianb, Sep 25 '11 at 15:56

score 2 · Answer 1 · answered Sep 25 '11 at 15:51

2

I think your problem is variable case:

if($gtipo <> 0...

should be

if($gTipo <> 0...

in 2 places.

answered Sep 25 '11 at 15:51

Gus

7,289
2
26
23

True, variables are case-sensitive. `php -r '$Foo = 1; var_dump($foo);'` – Annika Backstrom Sep 25 '11 at 15:53
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 – Acchile Biagioli Sep 25 '11 at 16:00

Your Common Sense · Answer 2 · 2011-09-25T16:05:00.287

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 '11 at 16:05

answered Sep 25 '11 at 15:58

Your Common Sense

156,878
40
214
345

Tks for help Col. Shrapnel... but I will keep my original code structure. Simple and works. – Acchile Biagioli Sep 25 '11 at 16:02
1

I wouldn't call a bunch of useless operators "simple". your first condition is totally useless – Your Common Sense Sep 25 '11 at 16:07

score 0 · Answer 3 · answered Sep 25 '11 at 20:31

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.

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

3 Answers3