Can this code be shortend at all?

[tomkilbourn]

Hello,

Im using a small piece of php code to control a joomla module. The code works well and controls a Joomla module in up to four columns.

Im no good with coding php, but was wondering out of interest if the code can be shortend / streamlined in any way? Im just wondering for my own learning curve wether it could be shortend or is it fine as it is? -

$corners = array('squared','rounded','fixed','disabled');

$this->corners[0]['squared']=false;
$this->corners[0]['rounded']=false;
$this->corners[0]['fixed']=false;
$this->corners[0]['disabled']=false;

$legitimate_values = $params->get('firstcorners', 1);
if(in_array($legitimate_values,$corners))
{
$this->corners[0][$legitimate_values] = true;
} 

$this->corners[1]['squared']=false;
$this->corners[1]['rounded']=false;
$this->corners[1]['disabled']=false;

$legitimate_values = $params->get('secondcorners', 1);
if(in_array($legitimate_values,$corners))
{
$this->corners[1][$legitimate_values] = true;
} 

$this->corners[2]['squared']=false;
$this->corners[2]['rounded']=false;
$this->corners[2]['disabled']=false;

$legitimate_values = $params->get('thirdcorners', 1);
if(in_array($legitimate_values,$corners))
{
$this->corners[2][$legitimate_values] = true;
} 

$this->corners[3]['squared']=false;
$this->corners[3]['rounded']=false;
$this->corners[3]['disabled']=false;

$legitimate_values = $params->get('fourthcorners', 1);
if(in_array($legitimate_values,$corners))
{
$this->corners[3][$legitimate_values] = true;
} 

If it could be shortend,how could it be done?🙂

[Weedpacket]

If it's easy to figure out what it does, if it's easy to maintain, if it isn't crashingly slow or buggy or a resource hog, then it's fine as it is. I'd swap the variable names $corners and $legitimate_values, because as it stands it looks like $corners contains the list of legitimate values, and $legitimate_values contains the corner being checked for legitimacy.

[johanafm]

I agree on the name swapping and the other stuff in general. But when you do more or less the same thing four times, I believe it belongs in a loop.

$first = array('squared' => false, 'rounded' => false, 'fixed' => false, 'disabled' => false);
$others = array('squared' => false, 'rounded' => false, 'disabled' => false);
$this->corners = array($first, $others, $others, $others);
$cornerNames = array('firstcorners', 'secondcorners', 'thirdcorners', 'fourthcorners');
$cornersTotal = count($cornerNames);
for ($i = 0; $i < $cornersTotal; ++$i) {
	$legitimate_values = $params->get($cornerNames[$i], 1);
	if (in_array())
		$this->corners[$i][$legitimate_values] = true;
}

[tomkilbourn]

Thankyou for your replies. Yes, a loop is what I was looking for! I've replaced the code I had with the new code suggested above, but I get an error message -

Warning: Wrong parameter count for in_array() in xxxxxxxx/helper.php on line 165

:queasy:

[laserlight]

Yes, you need to modify johanafm's example so that it actually works.

[tomkilbourn]

Im affraid im not competent enough to modify it on my own, Ive always struggled with coding :o Im fine with HTML and CSS though 🙂

I had a little go, but im not sure if Im way off with it, the module is displaying without any error, but none of the different corner styles (rounded, squared, disabled etc) are showing, so clearly im not doing it correctly.

Here is what I have so far

$corners = array('squared','rounded','fixed','disabled');
$first = array('squared' => false, 'rounded' => false, 'fixed' => false, 'disabled' => false);
$second = array('squared' => false, 'rounded' => false, 'fixed' => false, 'disabled' => false);
$third = array('squared' => false, 'rounded' => false, 'fixed' => false, 'disabled' => false);
$fourth = array('squared' => false, 'rounded' => false, 'fixed' => false, 'disabled' => false);
$this->corners = array($first, $second, $third, $fourth);
$cornerNames = array('firstcorners', 'secondcorners', 'thirdcorners', 'fourthcorners');
$cornersTotal = count($cornerNames);
for ($i = 0; $i < $cornersTotal; ++$i) {
    $legitimate_values = $params->get($cornerNames[$i], 1);
  if(in_array($legitimate_values,$corners))
        $this->$corners[$i][$legitimate_values] = true;
}