0

I have a php file called choose.php where inside i echo some HTML i.e. a select element.

I am using PDO to populate the select element from a mysql database.

the code i have written works perfectly but when i put it into a function and try to call it i get an error telling me that i cannot declare said method again.

the code is thus:

echo        '<select>';

                $sql = "SELECT name FROM people";
                $res = $conn->prepare($sql );
                $res ->execute();

                while ( $row  = $res ->fetch() )
                {
                    echo '<option value = "' . $row['name '] . '">' . $row['name '] . '</option>';
                }

echo        '</select>';

in other words the function would look like this:

function getnames()
{
        $sql = "SELECT name FROM people";
        $res = $conn->prepare($sql );
        $res ->execute();

        while ( $row  = $res ->fetch() )
        {
          echo '<option value = "' . $row['name '] . '">' . $row['name '] . '</option>';
        }
}

Why cant i call the method inside the echoed select element?

echo        '<select>';
                     getnames();
echo        '</select>';

Also how would i accomplish this by placing the method in another php file to keep it tidy?

Improve this question
1
  • please paste exact error message Commented Feb 3, 2014 at 23:02

4 Answers 4

Reset to default
4

Why cant i call the method inside the echoed select element?

Because the method body references $conn, which is supposedly a global variable and not in scope inside the method body. You can verify that this is the problem (and "fix" it) with 

function getnames()
{
    global $conn;
    // the rest as before
}

Now, although this will make the problem go away, what you propose here is not a good way to organize things. There are several issues:

  • getnames uses a global variable ("invisible argument") -- note that you would not have had reason to ask this question if this had been corrected!
  • The name of the method is misleading -- it doesn't "get" something, it prints HTML.
  • The method is unusable for anything else other than its specific purpose -- if you wanted to do something else with the names (e.g. print a table) you would have to write another method.
  • You are interleaving straight HTML output (the <select> tag) with business logic (querying the database). It's better to do all the business logic up front (keep the results you need in variables) and then do the HTML all in one go.

All of the above are serious deficiencies of the chosen approach and none of them would be present in a well built application. I suggest that instead of making the problem go away you would be better served by refactoring the code to address these, and the problem will fix itself on the way.

Code Review would be an excellent place to ask a question along the lines of "I have this code and this recommendation -- how would I implement it properly?" if you need extra help.

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

2 Comments

Note, however, that using global variables normally isn't a suitable solution but the problem... so you migth want to change your data-flow. WHy not just add a parameter to the function that takes the connection resource?
@Jon Hi Jon this was the exact issue, i included the connection_class.php file which sets up the connection to the database at the top of the page and assumed it would be global because. You will have to forgive me as i have only been learning PHP for a little over a week now. thanks again. Also JohannesH. i will try this suggestion now
4

You are trying to access $conn variable which is not available in your function scope. To access $conn variable inside your function use global, like below:

global $conn;
Improve this answer

1 Comment

Or better yet, make $conn an pass-in var to the function: function getnames($conn) { Using globals is one of the easiest ways to hang yourself (or future maintainers)
0

How are you loading the file in which the getnames function is defined? Try using require_once and making sure it's not being included more than once - already defined means it's be defined and the file is being called again, hence trying to define it again

Improve this answer

Comments

0

If you're calling that same code multiple times on your page it will get very heavy to load. I would recommend just running it at the top of the page and putting the data to a variable, then echoing that variable at each location that you need it

So your code in the top of your page

$sql = "SELECT name FROM people";
$res = $conn->prepare($sql );
$res ->execute();
$outputData = '';
while ( $row  = $res ->fetch() ){
  $outputData .= '<option value = "' . $row['name '] . '">' . $row['name '] . '</option>';
}

Then

echo '<select>'.$outputData.'</select>';
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.