HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Determine value in range, inject to HTML to be used in CSS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
cssrangeusedvalueinjectdeterminehtml

Problem

I want to show users what package they are, based on how many items they have in their cart. The code works, but I'm wondering if anything can be improved.

function new_thing (){

//decleare variables
global $x_subs;
$lim01 = get_option(woomps_limit01);
$lim02 = get_option(woomps_limit02);
$lim03 = get_option(woomps_limit03);
$x = $x_subs;
$plugin_url =(plugins_url("/", __FILE__ ));

//set CSS class based on value of $x 
if  ($x >= 1 && $x = $lim02 && $x = $lim03) {
    $active1 = "fb-sub-deactive";
    $active2 = "fb-sub-deactive";
    $active3 = "fb-sub-active";
} else {
    $active1 = "fb-sub-deactive";
    $active2 = "fb-sub-deactive";
    $active3 = "fb-sub-deactive";
}

//generate the content that will be displayed
$content = 

{$x_subs}

EOD;

return $content;
}


.fb-sub-img-holder {
width: 95%;
margin: 0 auto;
}

.fb-img {
width: 30%;

.fb-sub-active {
width: 50%;
}
.fb-sub-deactive {
width: 20%;
}

Solution

Why is $lim01 never used? Is it always equal to 1?

Only one of the subscription packs applies, if at all. You should first make it clear which subscription pack applies.

$subscription_pack = (($x >= $lim03) ? 3 :
                     (($x >= $lim02) ? 2 :
                     (($x >= $lim01) ? 1 : NULL)));
# Note: inactive rather than deactive
$active1 = $active2 = $active3 = 'fb-sub-inactive';
if ($subscription_pack == 1) { $active1 = 'fb-sub-active'; }
if ($subscription_pack == 2) { $active2 = 'fb-sub-active'; }
if ($subscription_pack == 3) { $active3 = 'fb-sub-active'; }


I also suggest reworking the CSS so that you can drop the fb-sub-inactive class altogether.

Code Snippets

$subscription_pack = (($x >= $lim03) ? 3 :
                     (($x >= $lim02) ? 2 :
                     (($x >= $lim01) ? 1 : NULL)));
# Note: inactive rather than deactive
$active1 = $active2 = $active3 = 'fb-sub-inactive';
if ($subscription_pack == 1) { $active1 = 'fb-sub-active'; }
if ($subscription_pack == 2) { $active2 = 'fb-sub-active'; }
if ($subscription_pack == 3) { $active3 = 'fb-sub-active'; }

Context

StackExchange Code Review Q#108204, answer score: 3

Revisions (0)

No revisions yet.