Taking a value from a loop in a class method

PHP
1

Here’s my problem that I’ve been stumped on for the last few hours

I’m creating a class to create html forms with: https://gist.github.com/1966505

In the create_fields() method I loop through a multidimensional array to create the form fields

This is what I end up with:

 $input = '<tr>';
 $input .= "<th><label for=\\"$id\\">$desc</label></th>";
 $input .= '<td><'.$form.' ></td>';
 $input .= '</tr>';

var_dump($input);

string '<tr><th><label for="feed-name">The name of the feed:</label></th><td><input type="text"  name="affiliate_hoover_plugin_options[feedName]"  id="feed-name"  class="regular-text code "  maxlength="200"  value="" ></td></tr>' (length=220)

string '<tr><th><label for="url-name">The URL of the feed:</label></th><td><input type="text"  name="affiliate_hoover_plugin_options[urlName]"  id="url-name"  class="regular-text code "  maxlength="300"  value="" ></td></tr>' (length=216)

Great

So if I was to echo it, no problems

However, when I try to take the returned values and use them in another method they disappear.

It’s not a scope issue it is how I am collecting the data from the loop that is the root of the problem

I’m sure there must be an easy solution that has passed me by

2

I see that in your major switch statement (in create_fields()) you are using “echo”. That will output the value but not persist it in any way.
So there is nothing to return; which I can’t see a return statement anywhere in that method.

3

If I was to use return instead of echo then I would get is null:

var_dump($cont->individual_fields("text", "feedName", "feed-name", null, "The name of the feed:", "200", "YES"));

<pre xmlns="http://www.w3.org/1999/xhtml" dir="ltr" class="xdebug-var-dump"><font color="#3465a4">null</font>
</pre>

Using echo returns the HTML

There is clearly something fundamentally stupidly wrong with how I am looping through the values

Damn it

This is the first method:

 public function individual_fields($type, $name, $id, $class = null, $desc, $maxlength = null, $value = null,
        $select = null) {

// validation remved

        // create an array out of the parameter values
        $fields_essentials = array(
            'type' => $type,
            'name' => $name,
            'id' => $id,
            'desc' => $desc,
            'class' => $class,
            'maxlength' => $maxlength,
            'value' => $value,
            'select' => $select);

        // call the create_fields() methods
        $this->create_fields($fields_essentials);

    }

Which then passes the array to the second method which creates the individual HTML fields

private function create_fields($fields_essentials) {

        extract($this->config_settings());

        foreach ($fields_essentials as $key => $value) {

            if ($key === "type") {

                switch ($value) {

                    case 'text':
                        // text area

                        $form = 'input type="text" ';

                        foreach ($fields_essentials as $key => $value) {

                            if ($key === "name") {
                                $form .= " name=\\"{$option_name}[{$value}]\\" ";
                                $name = $value;
                            }
                            if ($key === "id") {
                                ($value !== null) ? $form .= " id=\\"{$value}\\" " : null;
                                $id = $value;
                            }
                            if ($key === "desc") {
                                $desc = $value;
                            }
                            if ($key === "class") {

                                if ($value !== null) {
                                    $form .= " class=\\"regular-text code {$value}\\" ";
                                } else {
                                    $form .= " class=\\"regular-text code \\" ";
                                }

                            }
                            if ($key === "maxlength") {
                                ($value !== null) ? $form .= " maxlength=\\"{$value}\\" " : null;
                            }
                            if ($key === "value") {

                                if ($value !== null && $value != "YES") {
                                    $form .= " value=\\"{$value}\\" ";
                                }
                                if ($value == "YES") {

                                    $form .= ' value="';
                                    $form .= isset($_POST[$option_name[$name]]) ? print $_POST[$option_name[$name]] : null;
                                    $form .= '"';

                                }

                            } // end if $key

                        } // end foreach

                        $input = '<tr>';
                        $input .= "<th><label for=\\"$id\\">$desc</label></th>";
                        $input .= '<td><'.$form.' ></td>';
                        $input .= '</tr>';



                      return $input;   break;
                      // I'M THE RETURN VALUE DO SOMETHING WITH ME!



                    case 'hidden':
                    case 'button':
                    case 'image':
                    case 'password':
                    case 'reset':
                    case 'submit':
                        // error here - there form inputs are not cattered for
                        die("You cannot use the individual_fields() method to create inputs for $key");
                        break;
                    default:
                        // error message here
                        die("Make sure the input type in the individual_fields() method is correct");
                        break;

                } // end switch statement

            } // end fi

        } // end foreach

    }
4

Well, your first code seems gibberish to me, not sure what that even is… your later post and what’s on github?

if you have to === a string compare, there’s something horrifically wrong with your strings… == is just fine.

But more important, you’re looping for no good reason – if all you want to operate on is ‘type’, and type is a associative key, what are you even looping for?!?


	private function create_fields($fields_essentials) {
	
		extract($this->config_settings());
		
		switch ($fields_essentials['type']) {
			
			case 'text':

There’s no reason to be looping through values you aren’t even using! Your inner loop makes even LESS sense… wait, what’s the contents of $field_essentials again?

You’ve got other oddball logic failings and strange code in there… like the multiple echo statements to do the job of one, slow double quotes doing the job of singles, (which are those $vars inside $form supposed to be evaluated or not?!? Looks like you’ve got some form of templating stuff on top with those curly brackets in there)… You also seem to be declaring a whole slew of local variables for no good reason – and I’m not even convinced you need a loop.

Also since extract is a COPY, and not a reference, assigning values to the vars extracted by it really isn’t gonna do a whole lot for you.

Even the ‘null’ checks are odd, since you should probably be using isempty since it will actually return set and non-null from foreach.

… and that’s before we even talk about ‘what the devil is this table code doing in here?’.

Are you planning on doing anything with those extracted vars? Just seems really wierd… if you want to actually save values to them, you’ll need them as a reference, NOT as extracted copies.

I’ll look deeper at the code, but I suspect you’ve got over-thought things a bit, and at the same time left out some important safety checks (like isset and empty).

5

Reading deeper, I think I see what it is you were trying to do… I might be way off base in following it, but I suspect this is what you meant to do:


	private function create_fields($fields_essentials) {
	
		extract($this->config_settings());
		
		if (isset($fields_essentials['type']) {
		
			switch ($fields_essentials['type']) {
			
				case 'text':
				
					return '
						<label for="'.$field_essentials['id'].'">
							',$field_essentials['desc'],'
						</label>
						<input type="text"',(
							isset($field_essentials['name']) && !empty($field_essentials['name'])) ?
							' name="'.$option_name.'['.$field_essentials['name'].']"' :
							''
						),(
							(isset($field_essentials['id']) && !empty($field_essentials['id'])) ?
							' id="'.$field_essentials['id'].'"' :
							''
						),' class="regular-text code',(
							(isset($field_essentials['class']) && !empty($field_essentials['class'])) ?
							' '.$field_essentials['class'] :
							''
						),'"'(
							(isset($field_essentials['maxlength']) && !empty($field_essentials['maxlength'])) ?
							' maxlength= '.$field_essentials['maxlength'] :
							''
						),(
							(isset($field_essentials['value']) && !empty($field_essentials['value'])) ?
							' value="'.(
								$field_essentials['value']=='YES' ?
								(isset($_POST[$option_name[$name]]) ? $_POST[$option_name[$name]] : '') :
								$field_essentials['value']
							) :
							''
						),' />
						<br />';

					case 'hidden':
					case 'button':
					case 'image':
					case 'password':
					case 'reset':
					case 'submit':
						// error here - there form inputs are not cattered for
						die("You cannot use the individual_fields() method to create inputs for $key");
					break;
					
					default:
						// error message here
						die("Make sure the input type in the individual_fields() method is correct");
					break;

			} // END switch ($fields_essentials['type'])

		} // END if isset -- you may want to add an ELSE here!

	} // END method create_fields

Naturally swinging an axe at the table for nothing stuff. Untested, probably a typo or two, but reduces the code logic and should run many times faster.

6

Thanks a lot for your “help”

I’ll come back and deal with your “gibberish” insult later but I haven’t got time now

But I will state is using double quotes instead of single quotes or using echo does not impact on the performance of a PHP script in any significant way, and anyway that was NOT what I was asking about OKAY!!

7

Don’t take it personally man – I really wasn’t able to follow your original posts meaning; call it a comprehension issue, call it an incomplete picture – but for me it was gibberish – and besides, you have to break things down so you can build them up; there are several logic failings in the code and sloppy code practices that could be cleaned up; any ONE of them could be causing issues… You don’t want help fixing them, what are you even asking for help FOR?!?

What do I mean by logic failings?


        foreach ($fields_essentials as $key => $value) {

            if ($key === "type") {

                switch ($value) {

                    case 'text':
                        // text area

                        $form = 'input type="text" ';

                        foreach ($fields_essentials as $key => $value) {

First foreach is for ‘nothing’ – since you’re searching for only ONE valid condition on a KEY. Iterating through an associative array for a specific KEY doesn’t make any sense – waste of code, waste of processing time. If you know the key you want to process, don’t loop blindly through an associative array – that’s what associative arrays are there to stop you from having to do – Just check if it’s set, then use it!

Second foreach would be causing ‘current pointer’ issues. Arrays have a ‘pointer’ of sorts to the ‘current’ record… reset puts it at the start, end puts it at the end, next moves to the next one… and foreach… uses it… so by doing a foreach on the same array twice, you’re short-circuiting the outer one… Now you’re looking for a key value, so that’s ‘ok’ in a manner of speaking, but it’s wasted logic…

Because again, it’s an associative array – you don’t loop through an associative array and run case on it’s key – you just access it by the key.

Which is how my example code turned your two loop and case statement with multiple variable overheads, into two IF and a return… dropping it from 3.37k of code with multiple variables you didn’t even need - to 1.9k

As to the other things like quotes – maybe it’s the machine language coder in me, maybe it’s having learned coding back when stuff like that mattered – but to me that’s just sloppy coding. It might only be 1-3% if that – but when it’s the difference between hitting the shift key or not hitting the shift key… why waste the extra keystroke AND make sloppy code?

But from your reaction, apparently you don’t actually want help or to make the code better or even functional. Gotcha.