The below code works perfectly but the if/else statements look so long and ugly. Is there a way I can avoid these statements?

CommissionTypeFilterVm is a list that contains 6 properties shown below whose values are either 1 or 0. I wanted to enable and disable the checkbox's based on the values of these properties:

CommissionType
CommissionTrials
OverrideType
OverrideTrials
BonusType
AdjustmentType

Here is the code:

   if (view != null) 
        {
            if (view.CommissionTypeFilterVm !=null && view.CommissionTypeFilterVm.length>0)
            {
                if (view.CommissionTypeFilterVm[0].CommissionType != 1) {
                    this.$missionType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$missionType.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].CommissionTrials != 1) {
                    this.$missionTrails.prop("checked", false).prop("disabled", true);
                } else {
                    this.$missionTrails.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].OverrideType != 1) {
                    this.$overrideType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$overrideType.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].OverrideTrials != 1) {
                    this.$overrideTrails.prop("checked", false).prop("disabled", true);
                } else {
                    this.$overrideTrails.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].BonusType != 1) {
                    this.$bonusType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$bonusType.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].AdjustmentType != 1) {
                    this.$adjustmentType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$adjustmentType.prop("disabled", false).prop("checked", true);
                }
            }
        }

The below code works perfectly but the if/else statements look so long and ugly. Is there a way I can avoid these statements?

CommissionTypeFilterVm is a list that contains 6 properties shown below whose values are either 1 or 0. I wanted to enable and disable the checkbox's based on the values of these properties:

CommissionType
CommissionTrials
OverrideType
OverrideTrials
BonusType
AdjustmentType

Here is the code:

   if (view != null) 
        {
            if (view.CommissionTypeFilterVm !=null && view.CommissionTypeFilterVm.length>0)
            {
                if (view.CommissionTypeFilterVm[0].CommissionType != 1) {
                    this.$missionType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$missionType.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].CommissionTrials != 1) {
                    this.$missionTrails.prop("checked", false).prop("disabled", true);
                } else {
                    this.$missionTrails.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].OverrideType != 1) {
                    this.$overrideType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$overrideType.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].OverrideTrials != 1) {
                    this.$overrideTrails.prop("checked", false).prop("disabled", true);
                } else {
                    this.$overrideTrails.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].BonusType != 1) {
                    this.$bonusType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$bonusType.prop("disabled", false).prop("checked", true);
                }
                if (view.CommissionTypeFilterVm[0].AdjustmentType != 1) {
                    this.$adjustmentType.prop("checked", false).prop("disabled", true);
                } else {
                    this.$adjustmentType.prop("disabled", false).prop("checked", true);
                }
            }
        }

• javascript
• jquery
• refactoring

Share Improve this question Follow edited Nov 29, 2012 at 18:33 Will Vousden 33.4k99 gold badges8888 silver badges9797 bronze badges asked Nov 29, 2012 at 18:27 Naveen ReddyNaveen Reddy 15333 gold badges99 silver badges2424 bronze badges 2

• 7 This would be better suited for codereview.stackexchange. – Kevin B Commented Nov 29, 2012 at 18:31
• Depending on what else is contained in view, you could use a for in loop to loop through all properties of view and update the corresponding field based on it's value. – Kevin B Commented Nov 29, 2012 at 18:32

Add a ment  | 

4 Answers 4

Sorted by: Reset to default 8

It may depend on your exact view.CommissionTypeFilterVm[0] object (what other properties does it have) but it looks like you try to do this :

var  = view.CommissionTypeFilterVm[0];
for (var key in ) { // if necessary check ownProperty
   this['$'+key.charAt(0).toLowerCase()+key.slice(1)]
      .prop({checked: [key]==1, disabled: [key]!=1});
}

if (view && view.CommissionTypeFilterVm && view.CommissionTypeFilterVm.length) {
    var vm = view.CommissionTypeFilterVm[0],
        props = ["CommissionType", "CommissionTrials", "OverrideType", "OverrideTrials", "BonusType", "AdjustmentType"];
    for (var i=0; i<props.length; i++) {
        var prop = props[i],
            bool = vm[prop] != 1,
            key = "$"+prop.charAt(0).toLowerCase()+prop.slice(1);
        this[key].prop({checked: !bool, disabled: bool});
    }
}

You could work around the ugly key manipulation if you'd named your properties respectively. You need to do so anyway as you have a typo in the trails/trials.

You can just create a static map and loop over it:

    if (view != null && view.CommissionTypeFilterVm != null && view.CommissionTypeFilterVm.length > 0) 
{
    var mapping = [ 
        { type: 'CommisionType', ele: '$misionType' },
        { type: 'CommissionTrials', ele: '$missionTrails' },
        { type: 'OverrideType', ele: '$overrideType' },
        { type: 'BonusType', ele: '$BonusType' }
    ];
    var self = this;

    $.each(mapping, function() {
        if (view.CommissionTypeFilterVm[0][this.type] !==1) {
            self[this.ele].prop("checked", false).prop("disabled", true);
        } else {
            self[this.ele].prop("disabled", false).prop("checked", true);
        }
    });
}

Hope that helps.

Maybe not efficient .. Try this

if (view != null) 
{
    if (view.CommissionTypeFilterVm !=null 
                    && view.CommissionTypeFilterVm.length>0)
    {
        var typeFilter = view.CommissionTypeFilterVm[0];
        var check = typeFilter.CommissionType === 0 ? true : false;
        this.$missionType.prop({ 'checked' : !check , 'disabled' : check });
        check = typeFilter.CommissionTrials === 0 ? true : false;
        this.$missionTrails.prop({ 'checked' : !check , 'disabled' : check });
        check = typeFilter.OverrideType === 0 ? true : false;
        this.$overrideType.prop({ 'checked' : !check , 'disabled' : check });
        check = typeFilter.OverrideTrials === 0 ? true : false;
        this.$missionType.prop({ 'checked' : !check , 'disabled' : check });
        check = typeFilter.BonusType === 0 ? true : false;
        this.$bonusType.prop({ 'checked' : !check , 'disabled' : check });
        check = typeFilter.AdjustmentType === 0 ? true : false;
        this.$adjustmentType.prop({ 'checked' : !check , 'disabled' : check });
    }
}