[PHP] IF empty else probleempje

[remy007]

Ik heb een function gemaakt voor het checken van volledigheid voor een registratie formulier, dit om te voorkomen dat er 'lege' gebruikers zichzelf kunnen registeren.

Op basis van wat het script aantreft zal hij 1 (doorgaan) of 0 (stop) moeten terug geven, probleem is alleen dat dit script momenteel continue aangeeft dat alles 1 is. Terwijl ik toch duidelijk een leeg formulier invul en opstuur.

Het function-script zie er als volgt uit:

PHP:

<?
    function SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_therms){
    
        if (!empty($sc_reg_username) || !empty($sc_reg_emailadres) || !empty($sc_therms)){
                                
            Return $SCContinueReg   = "1";
                        
        }
        else
        {
            
            if (empty($sc_reg_username)){
                                        
                $GLOBALS['SCFailRegUser1']  = "1";
                                            
            }
            
            if(empty($sc_reg_emailadres)){
                                            
                $GLOBALS['SCFailRegUser2']  = "1";
                                            
            }
            
            if(empty($sc_therms)){
                                
                $GLOBALS['SCFailRegUser3']  = "1";
                                    
            }
        
            Return $SCContinueReg   = "0";
        
        }
    
    }
?>

De echo in het onderstaande script echo $SCContinueReg; is als debug doeleinde, om te kijken welke waarde er precies terug geparced wordt...
Het bijbehorende front-end script:

PHP:

<?

                    SCSetMainTextWindowStart(SCProfileTextHeader, CurrentPage);

                        if(isset($_POST["sc_reg_submit"]) && isset($_POST['sc_reg_username'])){
                            
                            $sc_reg_username     = trim($_POST['sc_reg_username']);
                            $sc_reg_emailadres   = trim($_POST['sc_reg_emailadres']);
                            $sc_therms           = trim($_POST['sc_therms']);
                            
                            SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_therms);
                            
                        }
                        
                        if (isset($_POST["sc_reg_submit"]) && $SCContinueReg = "1" || isset($SCRequestUriExploded[3])){

                            if(isset($_POST["sc_reg_submit"]) && $SCContinueReg = "1"){
                            
                        echo $SCContinueReg;
                                //SCRegStarted("register", $_POST['sc_reg_submit'], $_POST['sc_reg_username'], $_POST['sc_reg_emailadres'], $_POST['sc_therms'], $SCCurrentTimeSec, $SCCurrentUserIP);
                            
                            }elseif($SCRequestUriExploded[3] == "activate"){
                                echo "Activate";
                                SCRegStarted("activate", "0", $SCRequestUriExploded[4], $SCRequestUriExploded[5], "0", $SCCurrentTimeSec, $SCCurrentUserIP);
                            }

                        }
                        else
                        {
                        
                            if ($SCLogonCheck == 0){
?>
                        <form method="post" action="/members/register" name="sc_form_reg">
                            
                            <table class="sc_reg_form">
                                <tr>
                                    <td<? if (isset($SCFailRegUser1)){ echo " class=\"fail_reg\""; } ?>>Gebruikersnaam</td>
                                    <td><input type="text" name="sc_reg_username"></td>
                                </tr>
                                <tr>
                                    <td<? if (isset($SCFailRegUser2)){ echo " class=\"fail_reg\""; } ?>>E-mail adres</td>
                                    <td><input type="text" name="sc_reg_emailadres"></td>
                                </tr>
                                <tr>
                                    <td<? if (isset($SCFailRegUser3)){ echo " class=\"fail_reg\""; } ?>>Akkoord met voorwaarden</td>
                                    <td><input type="checkbox" value="1" name="sc_therms"></td>
                                </tr>
                                <tr>
                                    <td colspan="2"><input type="submit" value="Submit" name="sc_reg_submit"></td>
                                </tr>
                            </table>
                            <div class="dhtmlgoodies_question">Algemene Voorwaarden</div>
                            <div class="dhtmlgoodies_answer">
                                <div>
                                blaat
                                </div>
                            </div>
                            
                        </form>
<?
                            
                            }
                            else
                            {
                            }
                        }
                        
                    SCSetMainTextWindowExit('1');
                    
?>

Hopelijk ziet iemand hier wat er zo fout is, heb al een en ander aan de settings/volgorden veranderd. Momenteel helaas nog niet opgelost...

Proberen kan je hier (akkoord met voorwaarden aanvinken, anders komt er een fout voorbij, los ik later op, worden eerst dit).

Alvast bedankt!

[Afvalzak]

Als je de $sc_therms weghaalt uit deze code:

code:

 if (!empty($sc_reg_username) || !empty($sc_reg_emailadres) || !empty($sc_therms)){

Moet het volgens mij wel werken.

[marco_balk]

Volgens mij zit er een denkfout in je IF:

PHP:

if (!empty($sc_reg_username) && !empty($sc_reg_emailadres) && !empty($sc_therms))

Waarom trouwens '0' of '1' returnen en niet TRUE of FALSE?

PS: voorwaarden zijn 'terms' niet 'therms'

[ Voor 21% gewijzigd door marco_balk op 07-02-2011 22:47 . Reden: Onzin verwijderd ]

[Afvalzak]

@marco_balk dat ligt eraan wat de TS wilt natuurlijk, als hij alle velden ingevuld wilt zien dan moet het zeker && zijn, anders is || voldoende lijkt mij?

Overigens is mijn antwoord ook niet kloppend zie ik net, maar als je in plaats van de Posts drie lege variabelen controleert werkt het dan wel?

[ReenL]

$SCContinueReg zit alleen in de scope van de functie.

met $GLOBALS['SCContinueReg'] haal je m daaruit.

Return $SCContinueReg = "0";
Doet dus eigenlijk niets nu behalve "0" returnen.

[remy007]

afvalzak schreef op maandag 07 februari 2011 @ 22:40:
@marco_balk dat ligt eraan wat de TS wilt natuurlijk, als hij alle velden ingevuld wilt zien dan moet het zeker && zijn, anders is || voldoende lijkt mij?

Overigens is mijn antwoord ook niet kloppend zie ik net, maar als je in plaats van de Posts drie lege variabelen controleert werkt het dan wel?

Wat ik wil bereiken, misschien in de TS niet geheel duidelijk geweest, bij deze dan alsnog. Ik wil kunnen controleren of alle drie de (verplichte) velden wel degelijk zijn ingevuld. Ongeacht of de rest wel is ingevuld.

ReenL schreef op maandag 07 februari 2011 @ 22:43:
$SCContinueReg zit alleen in de scope van de functie.

met $GLOBALS['SCContinueReg'] haal je m daaruit.

Return $SCContinueReg = "0";
Doet dus eigenlijk niets nu behalve "0" returnen.

Dat klopt, als de velden verkeerd ingevuld zijn, dan zou dit script 0 moeten returnen. Als alles goed is ingevuld zou deze 1 moeten doorgeven om naar stap 2 te komen (defensieve registratie van de gebruiker).

[ Voor 28% gewijzigd door remy007 op 07-02-2011 22:52 ]

[Afvalzak]

Waarom doe je niet gewoon return 0 dan ? En hem opvangen met $SCContinueReg = SCCheckSendVars(,,)

[Patriot]

Die hele code is fubar. Het feit dat die echo altijd 1 teruggeeft is omdat je geen vergelijking in je if hebt staan maar een toewijzing. Zie regel 15 (en 17):

PHP:

if (isset($_POST["sc_reg_submit"]) && $SCContinueReg = "1" || isset($SCRequestUriExploded[3])){

Die tweede, dus de expressie

$SCContinueReg = "1"

geeft altijd true omdat de string "1" door PHP behandeld zal worden als de integer 1 en uiteindelijk de boolean true.

De reden dat die echo van regel 19 überhaupt werkt is een bijwerking van deze enorme fout. Anders zou er waarschijnlijk niets geprint worden (tenzij deze in een stuk code wordt bewerkt die je hier niet hebt laten zien).

EDIT:

Overigens, als ik zo naar de website, de HTML en de door jou getoonde code kijk, dan valt het te overwegen om die "SC" prefix die je overal in je html en code terug laat komen gewoon achterwege te laten. Het voegt namelijk helemaal niets toe als het overal voor staat.

[ Voor 17% gewijzigd door Patriot op 07-02-2011 23:04 ]

[remy007]

Patriot schreef op maandag 07 februari 2011 @ 23:00:
Die hele code is fubar. Het feit dat die echo altijd 1 teruggeeft is omdat je geen vergelijking in je if hebt staan maar een toewijzing. Zie regel 15 (en 17):

PHP:

if (isset($_POST["sc_reg_submit"]) && $SCContinueReg = "1" || isset($SCRequestUriExploded[3])){

Die tweede, dus de expressie

$SCContinueReg = "1"

geeft altijd true omdat de string "1" door PHP behandeld zal worden als de integer 1 en uiteindelijk de boolean true.

De reden dat die echo van regel 19 überhaupt werkt is een bijwerking van deze enorme fout. Anders zou er waarschijnlijk niets geprint worden (tenzij deze in een stuk code wordt bewerkt die je hier niet hebt laten zien).

EDIT:

Overigens, als ik zo naar de website, de HTML en de door jou getoonde code kijk, dan valt het te overwegen om die "SC" prefix die je overal in je html en code terug laat komen gewoon achterwege te laten. Het voegt namelijk helemaal niets toe als het overal voor staat.

Heb voor de zekerheid 1 en 0 vervangen door respectievelijk GO en STOP, en de statements beter gescheiden (en terms weggehaald), nu komt nog GO te voorschijn, ipv het normale registratie formulier. Er gaat blijkbaar toch iets verkeerd in de if() statement...

PHP:

<?
    function SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_terms){
    
        if (!empty($sc_reg_username) || !empty($sc_reg_emailadres)){
                                
            Return "GO";
                        
        }
        else
        {
            
            if (empty($sc_reg_username)){
                                        
                $GLOBALS['SCFailRegUser1']  = "FAIL";
                                            
            }
            
            if(empty($sc_reg_emailadres)){
                                            
                $GLOBALS['SCFailRegUser2']  = "FAIL";
                                            
            }
            
            if(empty($sc_terms)){
                                
                $GLOBALS['SCFailRegUser3']  = "FAIL";
                                    
            }
        
            Return "STOP";
        
        }
    
    }
?>

[Voutloos]

Als zowel username als email gevuld moeten zijn, moet je geen or doen...

[NMe]

Je hebt een or in plaats van een and in je if staan en ik durf te wedden dat één van de twee gewoon niet empty is. Heb je al eens gewoon een var_dump eroverheen gegooid? Je weet wel, écht debuggen in plaats van alleen maar naar returnwaardes kijken.

[remy007]

NMe schreef op maandag 07 februari 2011 @ 23:33:
Je hebt een or in plaats van een and in je if staan en ik durf te wedden dat één van de twee gewoon niet empty is. Heb je al eens gewoon een var_dump eroverheen gegooid? Je weet wel, écht debuggen in plaats van alleen maar naar returnwaardes kijken.

Heb var_dump over $sc_reg_username en $sc_reg_emailadres gehaald, beide geven 0 string(0) "" string(0) "" terug.

Nou kijk er morgen wel ff verder naar, morgen ochtend weer werken. In ieder geval alvast bedankt voor de moeite! Kick topic wel, als ik er nog niet uit kom .

[ Voor 3% gewijzigd door remy007 op 07-02-2011 23:45 ]

[Verwijderd]

Ik heb het even snel voor je uitgewerkt. De code die je eerst had was nogal omslachtig dus ik heb het even herschreven. Ik hoop dat je er wat aan hebt.

PHP:

<?php
function SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_terms){
    
    if(empty($sc_reg_username)) {
        $GLOBALS['SCFailRegUser1'] = 1;
        return 1;
    }

    if(empty($sc_reg_emailadres)) {
        $GLOBALS['SCFailRegUser2'] = 1;
        return 1;
    }

    if(empty($sc_terms)) {
        $GLOBALS['SCFailRegUser3'] = 1;
        return 1;
    }

    return 0;
    
}
?>

Zoals je wel kunt zien returned deze functie 0 als alles ingevuld is en 1 als het niet zo is.

[NMe]

Ja. En tegelijkertijd stel je de gebruiker maar van één fot op de hoogte omdat je direct de functie verlaat.

[Verwijderd]

Fixed

PHP:

<?php
function SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_terms){
    $b_ret = 0;

    if(empty($sc_reg_username)) {
        $GLOBALS['SCFailRegUser1'] = 1;
        $b_ret = 1;
    }

    if(empty($sc_reg_emailadres)) {
        $GLOBALS['SCFailRegUser2'] = 1;
        $b_ret = 1;
    }

    if(empty($sc_terms)) {
        $GLOBALS['SCFailRegUser3'] = 1;
        $b_ret = 1;
    }

    return $b_ret;
    
}
?>

[Voutloos]

Return dan gewoon een array met fouten, en lege array/true indien geen fout en dump al dat global gedoe.

[remy007]

Verwijderd schreef op dinsdag 08 februari 2011 @ 09:40:
Fixed

PHP:

<?php
function SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_terms){
    $b_ret = 0;

    if(empty($sc_reg_username)) {
        $GLOBALS['SCFailRegUser1'] = 1;
        $b_ret = 1;
    }

    if(empty($sc_reg_emailadres)) {
        $GLOBALS['SCFailRegUser2'] = 1;
        $b_ret = 1;
    }

    if(empty($sc_terms)) {
        $GLOBALS['SCFailRegUser3'] = 1;
        $b_ret = 1;
    }

    return $b_ret;
    
}
?>

Bedankt voor het mee denken allemaal!

De code werkt nu, weliswaar iets aangepast.

Function:

PHP:

<?
    function SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_terms){
    
        $b_ret = 1;

        if(empty($sc_reg_username)) {
            $GLOBALS['SCFailRegUser1'] = 1;
            $b_ret = 0;
        }

        if(empty($sc_reg_emailadres)) {
            $GLOBALS['SCFailRegUser2'] = 1;
            $b_ret = 0;
        }

        if(empty($sc_terms)) {
            $GLOBALS['SCFailRegUser3'] = 1;
            $b_ret = 0;
        }

        return $b_ret;
    
    }
?>

Front-end code:

PHP:

<?

                    SCSetMainTextWindowStart(SCProfileTextHeader, CurrentPage);

                        if(isset($_POST["sc_reg_submit"]) && isset($_POST['sc_reg_username'])){
                            
                            $sc_reg_username    = trim($_POST['sc_reg_username']);
                            $sc_reg_emailadres  = trim($_POST['sc_reg_emailadres']);
                            $sc_terms           = trim($_POST['sc_terms']);
                            
                            $SCContinueReg      = SCCheckSendVars($sc_reg_username, $sc_reg_emailadres, $sc_terms);
                            
                        }
                        if (isset($_POST["sc_reg_submit"]) && $SCContinueReg == 1 || isset($SCRequestUriExploded[3])){

                            if(isset($_POST["sc_reg_submit"]) && $SCContinueReg == 1){
                            
                                SCRegStarted("register", $_POST['sc_reg_submit'], $_POST['sc_reg_username'], $_POST['sc_reg_emailadres'], $_POST['sc_terms'], $SCCurrentTimeSec, $SCCurrentUserIP);
                            
                            }elseif($SCRequestUriExploded[3] == "activate"){
                                echo "Activate";
                                SCRegStarted("activate", "0", $SCRequestUriExploded[4], $SCRequestUriExploded[5], "0", $SCCurrentTimeSec, $SCCurrentUserIP);
                            }

                        }
                        else
                        {
                        
                            if ($SCLogonCheck == 0){
?>
                        <form method="post" action="/members/register" name="sc_form_reg">
                            
                            <table class="sc_reg_form">
                                <tr>
                                    <td<? if (isset($SCFailRegUser1)){ echo " class=\"fail_reg\""; } ?>>Gebruikersnaam</td>
                                    <td><input type="text" name="sc_reg_username"></td>
                                </tr>
                                <tr>
                                    <td<? if (isset($SCFailRegUser2)){ echo " class=\"fail_reg\""; } ?>>E-mail adres</td>
                                    <td><input type="text" name="sc_reg_emailadres"></td>
                                </tr>
                                <tr>
                                    <td<? if (isset($SCFailRegUser3)){ echo " class=\"fail_reg\""; } ?>>Akkoord met voorwaarden</td>
                                    <td><input type="checkbox" value="1" name="sc_terms"></td>
                                </tr>
                                <tr>
                                    <td colspan="2"><input type="submit" value="Submit" name="sc_reg_submit"></td>
                                </tr>
                            </table>
                            <div class="dhtmlgoodies_question">Algemene Voorwaarden</div>
                            <div class="dhtmlgoodies_answer">
                                <div>
                                blaat
                                </div>
                            </div>
                            
                        </form>
<?
                            
                            }
                            else
                            {
                            }
                        }
                        
                    SCSetMainTextWindowExit('1');
                    
?>

Nu op naar de volgende uitbreiding van de security

[NMe]

Je zit nog steeds met die ranzige globals.

[Voutloos]

En PHP short open tags, overbodige if en else statements, divitis + onnodig table gebruik, en een slechte scheiding tussen code en layout, etc. etc.

Maar goed, dat zijn geen uberdringende zaken, dus die kan je nu in een rustig tempo gaan leren.

[ Voor 25% gewijzigd door Voutloos op 08-02-2011 18:58 ]

[apokalypse]

NMe schreef op dinsdag 08 februari 2011 @ 18:51:
Je zit nog steeds met die ranzige globals.

En PHP snapt true en false niet?

Voutloos schreef op dinsdag 08 februari 2011 @ 10:13:
Return dan gewoon een array met fouten, en lege array/true indien geen fout en dump al dat global gedoe.

En stiekem wil je ook een array als input, en dan heb genoeg aan een map

En die heeft PHP ook: array_map()

[ Voor 58% gewijzigd door apokalypse op 08-02-2011 19:18 ]

[remy007]

Voutloos schreef op dinsdag 08 februari 2011 @ 18:57:
En PHP short open tags, overbodige if en else statements, divitis + onnodig table gebruik, en een slechte scheiding tussen code en layout, etc. etc.

Maar goed, dat zijn geen uberdringende zaken, dus die kan je nu in een rustig tempo gaan leren.

Zo denk ik er zelf ook over uiteraard, ben er een paar jaar uit geweest in PHP, begin nu weer met de website heropbouw. Vind zelf persoonlijk de huidige code er al veel beter uit zien als de voormalige versies (uit 2004/2006) welke ik nog wel als naslag heb .