0

I have a PHP class called ResetPassword. Within this class is a switch which will return a different string containing the required HTML.

This is the code below (I have omitted a few of the cases for the sake of brevity, there is also a default case which returns NULL)

    public function get_display_block($displayblock) {
     switch ($displayblock) {
    case 'EnterNewPassword':
        $displayblock = '
                        <form action="resetpassword.php" method="post">
                        <fieldset class="login">
                        <legend>Enter your information in the form below to reset your password: </legend>
                        <div><label for="password1">Password :</label>
                        <p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>
                        <div><label for="password2"><span class="required">*</span>Retype Password :</label>
                        <p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>

                        <br />
                        </fieldset><br />
                        <br />
                        <div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>
                        </form>
                        ';
            return $displayblock;
        break;

    case 'THIS_ISNT_WORKING':
        $displayblock = '
                        <form action="changepassword.php" method="post">
                        <fieldset class="login">
                        <legend>Enter your information in the form below to reset your password: </legend>
                        <div><label for="password1">Old Password : </label>
                        <p><input type="password" name="oldpassword" size="40" maxlength="60" /></p> </div>
                        <div><label for="password1">New Password :</label>
                        <p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>
                        <div><label for="password2"><span class="required">*</span>Retype New Password :</label>
                        <p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>

                        <br />
                        </fieldset><br />
                        <br />
                        <div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>
                        </form>
                        ';
        return $displayblock;
     break;
}  // end of switch

} //end of method

When calling the class/switch on my main page, The below code works fine.

$reset = new ResetPassword(); 
$displayblock = $reset->get_display_block('EnterSecretAnswer');

However, when attempting to call THIS_ISNT_WORKING nothing displays. A var_dump on $displayblock returns NULL.

$reset = new ResetPassword();
$displayblock = $reset->get_display_block('THIS_ISNT_WORKING');
var_dump($displayblock);

If I call any of the other cases (the ones omitted previously) it returns the required HTML.

Can anyone see something I am missing here? Any help as usual would be greatly appreciated.

EDIT: I have updated the switch as requested below so that the return statement is outside the switch. The end result is still the same. I have also changed the default case to return a string ("abcd") just to verify that the default case wasn't be called, and it wasnt. Still returning NULL.

hakre
  • 193,403
  • 52
  • 435
  • 836
Nik
  • 201
  • 1
  • 3
  • 11
  • 2
    Also you really should separate your html from your core logic. Use a template class method to load your html block. – Lawrence Cherone Jun 10 '12 at 04:56
  • Your sample code [works for me](http://codepad.viper-7.com/4qGHyq). – nickb Jun 10 '12 at 04:59
  • Your sample code works fine, only problem with the function which I have to correct to make it run is get_display_block - I have to close function by an additional curly bracket "}". – deej Jun 10 '12 at 05:37
  • @LawrenceCherone - Thanks, I will look into the template class method. nickb, Dharmavir - Maybe a problem with the server I have uploaded it with, as the code is the exact same as was on localhost however onec uploaded it doesn't seem to work. *head ache* – Nik Jun 10 '12 at 08:03
  • @Dharmavir - My bad, I forgot the add the closing curly brace for the method. I have updated the code now to reflect. thanks. – Nik Jun 10 '12 at 08:07
  • You should add "var_dump($displayblock);" in the beginning of your get_display_block function to see whats going on there. – Eugene Jun 10 '12 at 08:13

4 Answers4

1

You should never use return statement within switch. Basically using return statement once in a function is known as a good practice per my knowledge.

deej
  • 2,536
  • 4
  • 29
  • 51
  • 2
    ...Because? Rationale? Benefits, caveats, exceptions, anything? If you can keep your [complexity down](http://en.wikipedia.org/wiki/Cyclomatic_complexity) I see no reason in having multiple-returns in a method. – Mike B Jun 10 '12 at 05:19
  • @MikeB: Yes, but apart from that rule, the general saying (and I think everybody starts with it when creating a new function) is, so return on the last line of a function. So once and at the end is a good starting point IMHO. – hakre Jun 10 '12 at 13:37
1
public function get_display_block($displayblock) {
 switch ($displayblock) {
    case 'EnterNewPassword':
         $displayblock = '
                    <form action="resetpassword.php" method="post">
                    <fieldset class="login">
                    <legend>Enter your information in the form below to reset your password: </legend>
                    <div><label for="password1">Password :</label>
                    <p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>
                    <div><label for="password2"><span class="required">*</span>Retype Password :</label>
                    <p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>

                    <br />
                    </fieldset><br />
                    <br />
                    <div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>
                    </form>
                    ';
    break;

   case 'THIS_ISNT_WORKING':
      $displayblock = '
                    <form action="changepassword.php" method="post">
                    <fieldset class="login">
                    <legend>Enter your information in the form below to reset your password: </legend>
                    <div><label for="password1">Old Password : </label>
                    <p><input type="password" name="oldpassword" size="40" maxlength="60" /></p> </div>
                    <div><label for="password1">New Password :</label>
                    <p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>
                    <div><label for="password2"><span class="required">*</span>Retype New Password :</label>
                    <p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>

                    <br />
                    </fieldset><br />
                    <br />
                    <div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>
                    </form>
                    ';
         break;
    }

  return $displayblock;
}

Update

Does this work:

   <?php

   $reset = new Reset;
   $displayblock = $reset->get_display_block('THIS_ISNT_WORKING');
   var_dump($displayblock);

   class Reset{
    public function get_display_block($displayblock) {
    switch ($displayblock) {
       case 'EnterNewPassword':
            $displayblock = '
                <form action="resetpassword.php" method="post">
                <fieldset class="login">
                <legend>Enter your information in the form below to reset your password: </legend>
                <div><label for="password1">Password :</label>
                <p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>
                <div><label for="password2"><span class="required">*</span>Retype Password :</label>
                <p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>

                <br />
                </fieldset><br />
                <br />
                <div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>
                </form>
                ';
       break;

      case 'THIS_ISNT_WORKING':
         $displayblock = '
                <form action="changepassword.php" method="post">
                <fieldset class="login">
                <legend>Enter your information in the form below to reset your password: </legend>
                <div><label for="password1">Old Password : </label>
                <p><input type="password" name="oldpassword" size="40" maxlength="60" /></p> </div>
                <div><label for="password1">New Password :</label>
                <p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>
                <div><label for="password2"><span class="required">*</span>Retype New Password :</label>
                <p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>

                <br />
                </fieldset><br />
                <br />
                <div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>
                </form>
                ';
            break;
       }

     return $displayblock;
   }


}

It returns a non-null response for me.

Mike Mackintosh
  • 13,917
  • 6
  • 60
  • 87
  • Have updated the code to reflect this (i.e moved the return out side of the switch). Still returning NULL. Thanks for the answer though. – Nik Jun 10 '12 at 08:05
  • I found the error, I hadnt included the ResetPassword.php in my config file. I would have thought that as it wasnt included, calling " 'EnterNewPassword'" wouldn't have worked, but for some reason EnterNewPassword returned what it was meant to yet THIS_ISNT_WORKING didnt. Very strange. Thank for very much for your help, very much appreciated. – Nik Jun 10 '12 at 13:24
0

I found the solution, and embarrassingly the error wasn't in the switch or the way I was calling it. I have a config file which is included in every page (didn't mention as didn't think it was relevant) which I had forgot to add the class.

The thing that confused me for so long is that I wouldn't have thought the other cases in the switch would work either, but they did.... Oh well.

Thanks for everyone that answered.

Nik
  • 201
  • 1
  • 3
  • 11
-1

Use it like this

function get_display_block($displayblock) {
    switch ($displayblock) {
        case 'EnterNewPassword':
            echo '<form action="resetpassword.php" method="post">' .
                 '<fieldset class="login">' .
                 '<legend>Enter your information in the form below to reset your password: </legend>' .
                 '<div><label for="password1">Password :</label>' .
                 '<p><input type="password" name="password1" size="40" maxlength="60" /></p> </div>'.
                  '<div><label for="password2"><span class="required">*</span>Retype Password :</label>'.
                  '<p><input type="password" name="password2" size="40" maxlength="60" /></p>  </div>'.
                  '<br />'.
                  '</fieldset><br />'.
                  '<br />'.
                  '<div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>'.
                  '</form>';
            break;
      }
}

And just do the same for your other cases. Though doing it like this can be a bit slopping, but you'll just need to use it then on your script by calling <? get_displaying_block($yourString); ?>

Bobby
  • 4,332
  • 5
  • 32
  • 39
  • Sorry, there was a curly bracket missing, simple error as I threw it together just to get the guy a basic idea of what he would need to do. – Bobby Jun 10 '12 at 05:49
  • No downvote from me? Thanks for the answer. I will try it as a function rather then a class and see if that helps, thanks again. – Nik Jun 10 '12 at 08:04
  • -1 Converting your class to a function definitely isn't the solution. – Mike B Jun 10 '12 at 18:52
  • I didn't downvote the first time. No one has actually identified the problem.. all the answers just refactor his code somehow to be something different. You changed his class to a function and echoed the content instead of returning it but never explained why. What's wrong with classes and returning strings? [Don't take it personally](http://meta.stackexchange.com/a/121351/172224) – Mike B Jun 10 '12 at 19:11
  • Than I apologize. This solution could still be easily formatted into a class. – Bobby Jun 10 '12 at 19:14