2

I have a a couple of different radio buttons which return an ethnicity and a gender. The script runs inside an internal application so rather than returning "boy", "girl" or "both" I get back 7707330, 7707333, and 7707336. Similar from the ethnicity radio button.

I then need to validate data based on the combonation of ethnicity and gender. This was a pretty simple task but I have ended up with 15 if statements! It all works as it should, but there must be a cleaner solution? 

function test(radioResults) {
    var1 = radioResults[0].toString();
    var2 = radioResults[1].toString();

    var roll = parseFloat(parent.roll);

    if (var2 == '7707330') {
        gender = 'boy';
    }
    if (var2 == '7707333') {
        gender = 'girl';
    }
    if (var2 == '7707336') {
        gender = 'both';
    }

    if (var1 == '7707341') {
        maori(gender);
    }
    if (var1 == '7707344') {
        pasifika(gender);
    }
    if (var1 == '7707347') {
        all(gender);
    }
}

function maori(gender) {
    //Maori 
    if (gender == 'boy') {
        ethnicity = parseFloat(parent.getMBoys);
        validation(ethnicity);
    }
    if (gender == 'girl') {
        ethnicity = parseFloat(parent.getMGirls);
        validation(ethnicity);
    }
    if (gender == 'both') {
        ethnicity = parseFloat(parent.getTotalM);
        validation(ethnicity);
    }
}

function pasifika(gender) {
    //Pasifika
    if (gender == 'boy') {
        ethnicity = parseFloat(parent.getPBoys);
        validation(ethnicity);
    }
    if (gender == 'girl') {
        ethnicity = parseFloat(parent.getPGirls);
        validation(ethnicity);
    }
    if (gender == 'both') {
        ethnicity = parseFloat(parent.getTotalP);
        validation(ethnicity);
    }
}

function all(gender) {
    //All
    if (gender == 'boy') {
        ethnicity = parseFloat(parent.getBoys);
        validation(ethnicity);
    }
    if (gender == 'girl') {
        ethnicity = parseFloat(parent.getGirls);
        validation(ethnicity);
    }
    if (gender == 'both') {
        ethnicity = parseFloat(parent.getTotalRoll);
        validation(ethnicity);
    }
}

function validation(ethnicity) {

    percent = ethnicity * 5 / 100;

    if (ethnicity - percent > roll || (ethnicity + percent < roll)) {
        parent.document.getElementById('CF_7784443').value = "FAIL";
    } else {
        parent.document.getElementById('CF_7784443').value = "PASS";
    }
}

How would I go about cleaning this up?

4

4 に答える 4

6

これがあなたの好みに合っているかどうかはわかりませんが、私にはよりクリーンなソリューションのように思えます. 最初にすべてのマッピングを個別にレイアウトします。次に、マッピングをparentの正しいプロパティに解決します。次に、取得したプロパティに対してvalidationを 1 回だけ呼び出します。ここにスケッチがあります：

// map var1 and var2 to gender and ethnicity
var genderMap = { "7707330" : "boy", "7707333": "girl", "7707336": "both" };
var ethnicityMap = { "7707341": "maori", "7707344": "pasifica", "7707347": "all" };

// map gender and ethnicity to the correct property of "parent"
var props = 
    { "boy": 
        {
            "maori": "getMBoys", "pacifica": "getPBoys", "all": "getBoys"
        },
        "girl": 
        {   
            "maori": "getMGirls", "pacifica": "getPGirls", "all": "getGirls"
        },
        "all":
        {   
            "maori": "getTotalM", "pacifica": "getTotalP", "all": "getTotalRoll"
        }
    };

// get the correct property    
var prop = props[genderMap[var1]][ethnicityMap[var2]];

// run the validation
validation(parseFloat(parent[prop]));
于 2012-04-26T03:56:25.613 に答える
2

使用switchすると役立つ場合があります

switch (var2) {  
   case '7707330': gender = 'boy'; break;  
   case '7707333': gender = 'girl'; break;  
   case '7707336': gender = 'both'; break;
   default: gender = 'not boy or girl here?';
}

MDN のドキュメントはこちら

編集：改行を縮小します。

于 2012-04-26T03:46:03.437 に答える
1

このようにコーディングします。ステートメントを提案する人もいるかもしれswitchませんが、読みやすさに影響を与えずに行数を増やすため、コードでそれらを正直に使用したことはありません。

function foo() {
  var func;

  if (var1 == '7707330') {
    func = maori;
  } else if (var1 == '7707344') {
    func = pasifika;
  } else if (var1 == '7707347') {
    func = all;
  }

  if (var2 == '7707330') {
    func('boy');
  } else if (var2 == '7707333') {
    func('girl');
  } else if (var2 == '7707336'):
    func('both');
  }
}

function maori(gender) {
  //Maori 
  if (gender == 'boy') {
    ethnicity = parseFloat(parent.getMBoys);
  } else if (gender == 'girl') {
    ethnicity = parseFloat(parent.getMGirls);
  } else if (gender == 'both') {
    ethnicity = parseFloat(parent.getTotalM);
  }

  validation(ethnicity);
}

function pasifika(gender) {
  //Pasifika
  if (gender == 'boy') {
    ethnicity = parseFloat(parent.getPBoys);
  } else if (gender == 'girl') {
    ethnicity = parseFloat(parent.getPGirls);
  } else if (gender == 'both') {
    ethnicity = parseFloat(parent.getTotalP);
  }

  validation(ethnicity);
}

function all(gender) {
  //All
  if (gender == 'boy') {
    ethnicity = parseFloat(parent.getBoys);
  } else if (gender == 'girl') {
    ethnicity = parseFloat(parent.getGirls);
  } else if (gender == 'both') {
    ethnicity = parseFloat(parent.getTotalRoll);
  }

  validation(ethnicity);
}

function validation(ethnicity) {
  var percent = ethnicity * 5 / 100;

  if (ethnicity - percent > roll || (ethnicity + percent < roll)) {
    parent.document.getElementById('CF_7784443').value = "FAIL";
  } else {
    parent.document.getElementById('CF_7784443').value = "PASS";
  }
}
于 2012-04-26T03:51:54.330 に答える
0

さて、クリーンなコードの観点から、Switchステートメントのifを交換できます。説明は次のとおりです。

http://www.w3schools.com/js/js_switch.asp

まあそれはあなたが得ることができる最高の代替品です。もちろん、あなたのコードも有効です。

于 2012-04-26T03:48:52.113 に答える