Как можно упростить этот код JavaScript?

359

16 января 2017, 22:12

function change() {
    var input1 = document.getElementById("input1").value,
        input2 = document.getElementById("input2").value,
        sum = 50;
        if (input1 < 0){
          document.getElementById('input1').classList.add('error-input');
        }
        else if (input2 < 0){
          document.getElementById('input2').classList.add('error-input');
        }
        else{
          document.getElementById("summ").value = (input1 * input2)* sum;
          document.getElementById('input1').classList.remove('error-input');
          document.getElementById('input2').classList.remove('error-input');
        }
  }

Answer 1

Код неплохой, но единственное, что я сделал бы:

function change() {
  var i1 = document.getElementById("input1"),
      i2 = document.getElementById("input2"),
      sum = 50;
  if (i1.value < 0){
    i1.classList.add('error-input');
  }else if (i2.value < 0){
    i2.classList.add('error-input');
  }else{
    document.getElementById("summ").value = (i1.value * i2.value)* sum;
    i1.classList.remove('error-input');
    i2.classList.remove('error-input');
  }
}

Answer 2

Ну держи. Самое короткое что смог придуматью Прошу заметить, что весь твой код поместился в одну строчку, буквально в несколько слов.

<!doctype html> 
<html lang="en" ng-app> 
  <head> 
    <meta charset="utf-8"> 
    <title>My HTML File</title> 
    <script src='https://ajax.googleapis.com/ajax/libs/angularjs/1.6.1/angular.min.js'></script> 
  </head> 
  <body> 
     <input type="text" ng-model='input1' placeholder='input1'> 
     <input type="text" ng-model='input2' placeholder='input2'> 
     <p>input1*input2*50 = {{  input2 ?  +input1*+input2*50 : input1 ? 'Введите число в input2!' : 'Введите число в input1!'}}</p> 
  </body> 
</html>

Answer 3

Например, так:

function change() {
    var input1 = document.getElementById("input1"),
        input2 = document.getElementById("input2"),
        sum = 50;
    if (input1.value < 0) {
        input1.classList.add('error-input');
    }
    else if (input2.value < 0) {
        input2.classList.add('error-input');
    }
    else {
        document.getElementById("summ").value = (input1.value * input2.value) * sum;
        input1.classList.remove('error-input');
        input2.classList.remove('error-input');
    }
}

Посмотрите, у вас везде используется однотипные конструкции вида document.getElementById("input1") - и это можно вынести в переменные -- улучшается читаемость кода, т.к. строки короче и проще понимать.

В остальном, как по мне улучшать особо нечего. Разве что кардинально менять скрипт, раздробив его на отдельные функции, что-то типа:

function change() {
    var input1 = document.getElementById("input1");
    var input2 = document.getElementById("input2");
    var koeff = 50;
    check_input(input1);
    check_input(input2);
    if (input1.value > 0 && input2.value > 0) {
        document.getElementById("summ").value = (input1.value * input2.value) * koeff;
    }
}
function check_input(input) {
    if (input.value < 0) {
        input.classList.add('error-input');
    } else {
        input.classList.remove('error-input');
    }
}

PS Непонятно, что за домножение на 50 идёт. Если это какой-то коэффициент -- его так и нужно называть. Если он не нужен -- тогда его нужно совсем выпилить.