what is the wrong with this PHP function?

Question

can anybody tell me what is the wrong with this function?

I wrote this this function to get errors from $reg_errors array and to use those errors in different place in the same script.

function testimonialErrors ($reg_errors) {
    if ( !empty($reg_errors) ) {
            foreach ( $reg_errors AS $error) {
                echo "<li>$error</li>";
            }                                       
    }
    return $error; 
}

Then I called that function like this..

if ( !empty($reg_errors) ) {
    echo '<div class="error">
                <img src="images/error.png" />
                <h3>Errors,</h3>
                <ul>';
                echo testimonialErrors($reg_errors);
        echo '</ul>
            </div>';
}

But this code is not working.

UPDATE : this is my new code

function tesimonialErrors ($reg_errors) {
    if ( !empty($reg_errors) ) {
            foreach ( $reg_errors AS $error) {
                echo "<li>$error</li>";
            }                                       
    }
    return $error; 
}

And called it like this

if ( !empty($reg_errors) ) {
    echo '<div class="error">
                <img src="images/error.png" />
                <h3>Errors,</h3>
                <ul>';
                tesimonialErrors($reg_errors);
        echo '</ul>
            </div>';
}

@devOp no he doesn't. The echo is already inside the function. Your function can't see `$reg_errors`/ — BenM, Feb 07 '13 at 12:56
Learn what a *variable scope* is: http://php.net/manual/en/language.variables.scope.php — deceze, Feb 07 '13 at 12:57
I tried echoing function but not working..echo tesimonialErrors(); — TNK, Feb 07 '13 at 12:58
@jeroen Actually what has happened there? I am trying this for hours to fix. but still no luck — TNK, Feb 07 '13 at 13:28

score 4 · Answer 1 · answered Feb 07 '13 at 12:57

4

You should look into variable scope. At this moment, $reg_errors is not defined in the scope of your function. You could pass it to the function:

function tesimonialErrors ($reg_errors) {
  ...

and call it like:

tesimonialErrors($reg_errors);

answered Feb 07 '13 at 12:57

jeroen

91,079
21
114
132

And don't forget to put it in the call ... :) – Bart Friederichs Feb 07 '13 at 12:58
3

Better this way than using globals! – Mez Feb 07 '13 at 12:58
@jeroen I tried it but not working. Check I updated my question – TNK Feb 07 '13 at 13:05
@Tharanga Nuwan You should leave your original code and put your edits below for clarity. Apart from that, the edited code seems about right, what does `$reg_errors` contain and what is the exact html output? – jeroen Feb 07 '13 at 13:09
@Tharanga Nuwan By the way, you should not `echo` the output of your function (just call it, you are echoing in the function itself), nor do you need to return the last `$error` value; that will mess up your list / html. – jeroen Feb 07 '13 at 13:11
@jeroen I printed $reg_errors then I got this - Array ( [name] => Name : This field is required. [email] => Email Address : This field is required. [membertype] => Membership Type : This field is required. [0] => No file selected [write-testimonials] => Testimonial : This field is required. ) – TNK Feb 07 '13 at 13:12
@Tharanga Nuwan That should work, just get rid of the extra `echo` in your main code. – jeroen Feb 07 '13 at 13:15

score 0 · Answer 2 · answered Feb 07 '13 at 14:12

Why not do it like this?

if ( !empty($reg_errors) ) {
    echo '<div class="error">
            <img src="images/error.png" />
            <h3>Errors,</h3>
            <ul>
               <li>';
    echo implode('</li><li>', $reg_errors);
    echo '    </li>
            </ul>
        </div>';
}

score -1 · Answer 3 · answered Feb 07 '13 at 13:00

As @Akam pointed out, $reg_errors is in the global scope so you need to use either global $reg_errors; or $GLOBALS['reg_errors'].

Also I'd suggest changing the function name tesimonialErrors to proper English (I'd assume that would be testimonialErrors). Otherwise, The Next Guy might end up posting it on TDWTF.

score -1 · Answer 4 · edited Feb 07 '13 at 14:05

-1

function testimonialErrors ($reg_errors) {
    if ( !empty($reg_errors) ) {
            foreach ( $reg_errors AS $error) {
                echo "<li>$error</li>";
            }                                       
    }
    return $error;  // ****remove return statement**** 
}

edited Feb 07 '13 at 14:05

Grambot

4,370
5
28
43

answered Feb 07 '13 at 13:47

Muhammad Ali

17
3

powtac · Answer 5 · 2013-02-07T14:16:36.197

-2

Add global $reg_errors; to your function.

function testimonialErrors () {
    global $reg_errors;
    if (!empty($reg_errors)) {
        foreach ($reg_errors as $error) {
            echo "<li>$error</li>";
        }
    }
}

edited Feb 07 '13 at 14:16

answered Feb 07 '13 at 12:57

powtac

40,542
28
115
170

3

Globals are evil! - http://stackoverflow.com/questions/5166087/php-global-in-functions have a read :) – Mez Feb 07 '13 at 12:58
@Mez: why? could you send a reference? – Feb 07 '13 at 13:00
@Mez: I'd agree in principle but the OP didn't seem like they were likely or willing to change that much of their code. – Magnus Feb 07 '13 at 13:06

what is the wrong with this PHP function?

5 Answers5