Is there a better way to do this?

[nloding]

I'm struggling with finding some cleaner logic for this.  I'm passing an array to function that is drawing a text input for a form.  $array['class'] can equal the class to be assigned to this input; however, if there's an error in the form processing, it needs the class 'form-text-error' also.  So ... if there's a 'class' array key, I might need to append 'form-text-error' if there's an error.

 

Can anyone see anything cleaner than this?  I hate 'dirty' code and try to keep what I write as concise and clean as I can.  To me, this seems kinda clunky and the OCD/perfectionist in me says it can be better.  I've been staring at it for three days and figured I'd see if anyone here had any ideas.

 

<?php
	$class = false;
	if(is_array($extras)) {
		foreach($extras as $key=>$value) {
			if($key != 'text' && $key != 'position' && $key != 'class')
				$radio .= ' ' . $key . '="' . $value . '"';
			if($key == 'class') $class = $value;
		}
	}

	if($class) $radio .= ' class="' . $class;
	if($this->hasError) {
		if(array_key_exists($name, $this->errors)){
			if(!$class) $radio .= ' class="form-radio-error"';
			else $radio .= ' form-radio-error"';
		} else {
			$radio .= '"';
		}
	} else {
		$radio .= '"';
	}
?>

[deadimp]

You could clean it up some, making $class the only variable that controls what the CSS class will be (outputting it at the end):

$radio=?; //What is radio? Couldn't this apply to any field / form element?
$class='';
if (is_array($extras)) {
$check=array('text','position','class'); //This is a little less efficient that the way you have it below, but a little more flexible.
foreach ($extras as $key => $value) {
 if (!in_array($key,$check)) $radio=" $key=\"$value\"";
 if ($key=='class') $class=$value;
}
}
if ($this->hasError) {
if (array_key_exists($name,$this->errors)) $class.=' form-radio-error';
else $class=''; //Don't add anything else?
}
$class=trim($class); //Get rid of the extra whitespace - OCD on formatting 
if ($class) $radio.=" class=\"$class\"";

There's alternate ways to change it. You could just have all of the field's attributes controlled by the array $extras, and on errors have it manipulate the element 'class' (adding it if it doesn't exist) in that array. That might be a little more flexible, and you could divide some of the functionality with that.

Now, what's the point of having it specify 'text' and 'position'? 'text' would just be 'value', and I really don't know what 'position' could be. You could have a sub-array/string in that one array that's style, which either lists the attributes in an array or string.

Ex:

$extras=array(
'name'=>'bob',
'size'='10',
'value'=>'Awesomeness',
'style'=>array(
 'text-align'=>'left',
 'margin'=>'16px')
);

Applying your own formatting, of course.

[nloding]

The 'text' and 'position' values had to do with the text displayed next to the radio/checkbox.  The function I posted was for a radio input.  The position is either left or right of the radio, and the text is what's displayed:

 

<input type="radio" value="action_add" name="action" /> Add

 

'Add' would be the 'text' and 'right' would be the 'position'.  I find myself not using labels for radios/checkboxes, though I probably should.  I usually use the label for the "grouping", not the individual checkboxes/radios.  Later I'll add the 'label' option in there.  In my <textarea> function, for instance, you can add a label and set the position as 'left', 'right', 'above', or 'below'.

 

I like the idea of the excluded keywords in an array -- that makes it easier to add/subtract later in the game.