Вопрос

Just in case anyone was not familiar with Joomla, countModules is a function built into it. Also keep in mind I am NOT very familiar with PHP.. and I'm rather new to using it. Please try to keep any solutions simple for understanding. Thanks.

My questions is could I condense this:

if($this->countModules('Top1A and Top1B') >= 1) $modCellRow1 = "col2";
if($this->countModules('Top1B and Top1C') >= 1) $modCellRow1 = "col2";
if($this->countModules('Top1A and Top1C') >= 1) $modCellRow1 = "col2";

into this:

if ($this->countModules('Top1A and Top1B') >= 1) || ($this->countModules('Top1A and Top1B') >= 1) || ($this->countModules('Top1A and Top1C') 
{
$modCellRow1 = "col2";
}

If you do know Joomla and countModules... or even if you don't, keep in mind I am checking to see if there is anything in each of the module positions rather then counting how many actual modules there are. In other words, I DO need to use AND as an operator. Also, I don't want to include all three because that is another line in my code... for example:

if($this->countModules('Top1B and Top1B and Top1C') >= 1) $modCellRow1 = "col3";

What I'm checking on those other statements is whether there is something in just TWO of the module positions... (I.E. only two of the three that are equal to or greater the one... but not all three).

ADDITIONAL COMMENT / QUESTION:

Would this work then?

$topRow1Count = 0;
foreach(array('Top1A','Top1B','Top1C','Top1D') as $position) {
    if ($this->countModules($position)) {$colCount += 1;}
}
$modColCountTopRow1 = "col" . $topRow1Count;
if ($topRow1Count == 0) { $modTopRow1 = " hidden"; }

$topRow2Count = 0;
foreach(array('Top2A','Top2B','Top2C','Top2D') as $position) {
    if ($this->countModules($position)) {$colCount += 1;}
}
$modColCountTopRow2 = "col" . $topRow2Count;
if ($topRow2Count == 0) { $modTopRow2 = " hidden"; }

if ($topRow1Count + $topRow2Count == 0) { $topModCont1 = " hidden"; }
Это было полезно?

Решение

Yes it will work. But let's consider performance and readability.

Performance

You are invoking countModules 3 times for the "col2" case, once for the "col3" case assuming you are starting with "col1" it will be 4 invocations,each with multiple "positions" to check for. Now the countModules() code is pretty optimized, falling back to a single static class that will not query the db at every iteration; but you're still making your program 10 times slower than it should be even in a small site with few modules.

Readability & code quality

You might have read that code should not look like it's copied and pasted. That makes sense at least because it's easier to read code that avoids duplication, additionally it will be less prone to typos.

In the spirit of condensing multiple statements, instead of joining lines, why not consider:

$colCount = 0;
foreach(array('Top1A','Top1B','Top1C') as $position) {
    if ($this->countModules($position)) {
        $colCount += 1;
    }
}
$modCellRow1 = "col" . $colCount;

This will return "col0" if no modules are in any of the positions, "col1" if modules are present only in one and so on.

I wrote it so it's easiest to read,

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top