Как правильно оформить код?

Question

[Smirator]

Привет! Есть код, со множеством if else. Работает норм, но выглядит так себе. Есть вариант оформить правильнее? Особенность, в том, что это в верстке используются select
Гуглил, но толкового решения для своей ситуации не нашел.

function selectChange(){
  let languageSelection = document.forms.dynamicSurvey.languageSelection;
  let choiseLanguage = document.forms.dynamicSurvey.lang;
  let print = document.forms.dynamicSurvey.print;
  let typePrint = document.forms.dynamicSurvey.typePrint;
  let receivingOptions = document.forms.dynamicSurvey.receivingOptions;
  let town = document.forms.dynamicSurvey.town;
  let file = document.forms.dynamicSurvey.file;
  let printColor = document.forms.dynamicSurvey.printColor;
  let fromLang = document.forms.dynamicSurvey.fromOtherLang;
  let inLang = document.forms.dynamicSurvey.inOtherLang;
  let notaryDoc = document.getElementById('notaryDoc');


  let questionZero = document.getElementById('question-0');
  let questionOne = document.getElementById('question-1');
  let questionTwo = document.getElementById('question-2');
  let questionThree = document.getElementById('question-3');
  let questionThreeSecond = document.getElementById('question-3-1');
  let questionFour = document.getElementById('question-4');
  let questionFive = document.getElementById('question-5');
  let questionSix = document.getElementById('question-6');
  let questionSeven = document.getElementById('question-7');
  let questionOther = document.getElementById('question-other');

  let answerOne = document.getElementById('answer-1');
  let answerTwo = document.getElementById('answer-2');

  let promptOne = document.getElementById('prompt-one');
  let promptTwo = document.getElementById('prompt-two');

  let nameUser = document.getElementById('name-text');
  let wishesUser = document.getElementById('wishes-text');
  let addressUser = document.getElementById('address-text');
  let anotherLanguageOne = document.getElementById('anotherLanguageOne');
  let anotherLanguageTwo = document.getElementById('anotherLanguageTwo');

  if(languageSelection.value === 'fromRus' || languageSelection.value === 'inRus'){
    anotherLanguageOne.classList.add('d-n');
    anotherLanguageTwo.classList.add('d-n');
    questionZero.classList.remove('d-n');
    choiseLanguage.classList.remove('d-n');
    questionOther.classList.add('d-n');
    fromLang.classList.add('d-n');
    inLang.classList.add('d-n');
    if(languageSelection.value === 'fromRus'){
      notaryDoc.classList.remove('d-n');
    } else {
      notaryDoc.classList.add('d-n');
      file.selectedIndex = 0;

    }
    if(choiseLanguage.value){
      questionOne.classList.remove('d-n');
      print.classList.remove('d-n');
      if(print.value === 'yes'){
        answerOne.classList.add('d-n');
        questionTwo.classList.remove('d-n');
        typePrint.classList.remove('d-n');
        if(typePrint.value){
          questionThree.classList.remove('d-n');
          receivingOptions.classList.remove('d-n');
          if(receivingOptions.value === 'inPaper' || receivingOptions.value === 'paperAndEl'){
          answerTwo.classList.add('d-n');
          addressUser.classList.add('d-n');
          questionThreeSecond.classList.remove('d-n');
          town.classList.remove('d-n');
          if(town.value === 'saint-peterburg' || town.value === 'moscow'){
              questionFour.classList.remove('d-n');
              file.classList.remove('d-n');
              if(file.value === 'doc' || file.value === 'notaryDoc'){
                questionFive.classList.add('d-n');
                questionSix.classList.remove('d-n');
                nameUser.classList.remove('d-n');
                printColor.classList.add('d-n');
                  questionSix.classList.remove('d-n');
                  nameUser.classList.remove('d-n');
                  questionSeven.classList.remove('d-n');
                  wishesUser.classList.remove('d-n');
                  if(languageSelection.value === 'fromRus'){
                    promptOne.classList.remove('d-n');
                    promptTwo.classList.add('d-n');
                  } else if(languageSelection.value === 'inRus' || languageSelection.value === 'other'){
                    promptOne.classList.add('d-n');
                    promptTwo.classList.remove('d-n');
                  }
              } else if(file.value === 'easyDoc'){
                questionFive.classList.remove('d-n');
                printColor.classList.remove('d-n');
                if(printColor.value){
                  questionSix.classList.remove('d-n');
                  nameUser.classList.remove('d-n');
                  questionSeven.classList.remove('d-n');
                  wishesUser.classList.remove('d-n');
                  if(languageSelection.value === 'fromRus'){
                    promptOne.classList.remove('d-n');
                    promptTwo.classList.add('d-n');
                  } else if(languageSelection.value === 'inRus' || languageSelection.value === 'other'){
                    promptOne.classList.add('d-n');
                    promptTwo.classList.remove('d-n');
                  }
                }
              }
          } else if(town.value === 'anCity'){
            questionFour.classList.add('d-n');
            file.classList.add('d-n');
            questionFive.classList.add('d-n');
            printColor.classList.add('d-n');
            answerTwo.classList.remove('d-n');
            addressUser.classList.remove('d-n');
            questionFour.classList.remove('d-n');
            file.classList.remove('d-n');
          }

Полотно из условий продолжается, но писать его полностью не стал. Подскажите, какой подход стоит применить в этом случае?

Comments

[WbICHA]

Есть вариант оформить правильнее?

Есть, если вообще всё рефакторнуть, включая хтмл.

Answer 1

[dollar]

Применяется подход, я бы его назвал "отбрасывание хвоста", но возможно есть и официальное название. Работает внутри функции в виде досрочного выхода из неё.

При if и if-else нужно как бы помнить все предыдущие if и держать их в голове. При отбрасывании хвоста всё наоборот - разум очищается, отработанные мысли выкидываются, и в голове обычно 1-2 условия нужно держать.

Подход можно применить частично, потому что это может быть не тупая колбаса условий if, а else-ветвистая. Но каждую линейную ветвь точно можно превращать в маленький хвостик, а если рефакторить, то и ветки побольше. Главное, всё время стараться избавиться от лишнего ментального груза. Тогда код превращается в обычную прозу, где инструкции следуют одна за другой (императивно) без сложной многоступенчатой логики, и его становится легко читать и понимать.

Для if:

Конструкция

if (condition_A) {
  instruction_A;
  if (condition_B) {
    instruction_B;
    if (condition_C) {
      instruction_C;
      .....
    }
  }
}

Превращается в (вложенности уходят):

if (!condition_A) return;
instruction_A;
if (!condition_B) return;
instruction_B;
if (!condition_C) return;
instruction_C;
.....

Для if-else:

Конструкция

if (condition_A) {
  instruction_A;
} else if (condition_B) {
  instruction_B;
} else if (condition_C) {
  instruction_C;
} else if .....

Превращается в обычный switch, либо снова в отбрасывание хвоста:

if (condition_A) {
  instruction_A;
  return;
}
if (condition_B) {
  instruction_B;
  return;
}
if (condition_C) {
  instruction_C;
  return;
}
if .....

Сложные ветки можно выделять в отдельные функции, это тоже как бы граница логики, перешагивая через которую можно позволить себе забыть часть кода, хоть и временно. Дело в том, что return работает на всю функцию целиком, в этом и есть его сила. Но на отдельную ветку он не применим, пока не выделить её в отдельную функцию, - тогда снова можно применять.

Конечно, если есть повторы в условиях, как у вас, то можно как-то сообразить, как это представить в виде цикла (обхода массива любым способом), или скомбинировать разные ветки условий в одну с помощью && и ||, или опять-таки выделить в подфункцию. Сообразить = рефакторить. Обработку ошибок можно также оборачивать в try-catch, но это уже другая история.

Comments

[Smirator]

Большое спасибо за развернутый и содержательный ответ! Я проанализирую данный подход и попробую применить его в своей ситуации.

Answer 2

[GeminiX]

можно попробовать разбить на такие conditions

const conditions = [
    {
       condition: ( ) => true,
       action: ( ) => {
           console.log('action')
       } 
    }
]

conditions.filter((item) => item.condition( )).map((item)=> item.action( ))

Comments

[Smirator]

Благодарю!

[GeminiX]

Smirator, то, о чем dollar пишет, называется Guard Clause

[dollar]

Александр, спасибо)