Веб-приложение калькулятора BMR, написанное на HTML / CSS / Vanilla JS

В данный момент я изучаю JavaScript (все еще новичок), и это первое приложение, которое я разработал самостоятельно, без использования каких-либо руководств. Это приложение, которое принимает пол, возраст, рост и вес пользователя, а затем вычисляет их BMR. Это работает, но я был бы очень признателен, если бы кто-нибудь более знающий мог взглянуть на код, чтобы увидеть, оптимален ли он (или даже близок!). Я хочу как можно скорее отказаться от любых вредных привычек, которые у меня возникли.

Вот ссылка на весь код на GitHub, а также ReadMe, в котором подробно объясняется, как работает приложение:

https://github.com/neilmurrellcoding/bmr-calculator-version-one

HTML-код:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">

  <link rel="stylesheet" href="css/index.css">
  <title>BMI Calculator</title>
</head>
<body>
  <div class="first-container">
  <div class="second-container">
    <div class="application">
      <h1 class="title">DIET PLANNER</h1>
      <p class="intro">Fill out the below fields to discover your BMR.</p><br>
      <p class="intro">Your BMR is the number of calories you burn off in 24 hours</p>

      <form>
        <div class="radio-buttons">
          <p class="label"><strong>Select your gender</strong></p>
          <input type="radio" checked="checked" id="male" name="gender" value="male"><label for="male" class="gender-label">Male</label>
          <input type="radio" id="female" name="gender" value="female"><label for="female" class="gender-label">Female</label>
        </div>


        <p class="label"><strong>Age</strong></p>
        <input type="number" id="age" class="age-field" min="0" max="130" placeholder="Enter your age"><br><br><br>
        
        <p class="label"><strong>Height</strong></p>
        <input type="number" id="feet" class="field" min="0" max="9" placeholder="ft">
        <input type="number" id="inches" class="field" min="0" max="11" placeholder="in"><br>

        <p class="label"><strong>Weight</strong></p>

        <input type="number" id="stone" class="field" min="0" max="80" placeholder="stone">
        <input type="number" id="lbs" class="field" min="0" max="13" placeholder="lbs"><br>
        

        <input type="submit" id="submit" class="submit">
      </form>

      
    </div>

  </div>
  <div id="results">
    <div class="results-container">
      <h1 class="title">Your daily BMI is:</h1>
      <p id="bmi-result">Placeholder text</p>
    </div>
  </div>
</div>
  <script src="application.js"></script>
</body>
</html>

Код JS:

document.getElementById('results').style.display = 'none';

submit.addEventListener('click', function(e) {  // Runs getValues when form's submit button is clicked.
  e.preventDefault();

  getValues();
})

getValues = () => {  // Converts values from form fields into integers, then assigns these integers to variables.
const age = parseInt(document.getElementById("age").value);
const gender = document.getElementsByName("gender"); // This produces a node-list with 2 items, male and female.  Male is checked by default.
const heightFeet = parseInt(document.getElementById("feet").value);
const heightInches = parseInt(document.getElementById("inches").value);
const weightStone = parseInt(document.getElementById("stone").value);
const weightLbs = parseInt(document.getElementById("lbs").value);

x = calculateAge(age); // calls the calculateAge function below, passes 'age' as an argument, assigns return value to x.
y = calculateWeight(weightStone, weightLbs); // calls the calculateWeight function below, passes 'weightStone' & 'weightLbs' as args, assigns return value to y.
z = calculateHeight(heightFeet, heightInches); // calls the calculateHeight function below, passes 'heightFeet' & 'heightWeight' as args, assigns return value to z.
finalResult(x, y, z, gender); // calls finalResult function below, passes return values of above 3 functions as args. 
}


calculateAge = age => { // Multiplies user's age by 5 and saves this as finalAge.
  finalAge = age * 5;
  return finalAge;
}

calculateWeight = (weightStone, weightLbs) => { // Converts user's imperial weight into kg, multiplies it by 10, and returns finalWeight.
  kilogramWeight = ((weightStone * 14) + weightLbs) * 0.453;
  finalWeight = kilogramWeight * 10;
  return finalWeight;
}

calculateHeight = (heightFeet, heightInches) => { // Converts user's imperial height into cm, multiplies it by 6.25, and returns finalHeight.
  centimeterHeight = ((heightFeet * 12) + heightInches) * 2.54;
  finalHeight = centimeterHeight * 6.25;
  return finalHeight;
}

finalResult = (x, y, z, gender) => {
  result = z + y - x; // finalWeight + finalHeight - finalAge.
  for(i = 0; i < gender.length; i++) { // This checks to see which gender the user checked. If 'male', +5 to finalResult. Else, -161 from finalResult.
    if(gender[i].checked) {
      finalResult = result + 5;
      break;
    } else {
      finalResult = result - 161;
      break;
    }
  }
 
  revealResult(finalResult); // Calls below function & passes finalResult as an argument.

  return finalResult;
}

revealResult = finalResult => {
  finalResult = Math.floor(finalResult);
  let finalBmi = finalResult.toString()
  
  document.getElementById('results').style.display = 'block'; // Reveals the 'results' box which was originally hidden (see line 1)
  const calories = document.getElementById("bmi-result"); // Grabs 'p' element.
  calories.classList.add("finalNumberStyling") // Applies simple styling to 'p' element.
  calories.innerText = finalBmi + ' calories per day'; // Inserts finalBmi string into 'p' element.
}

Я не включил код CSS, потому что стиль очень минимален, но на всякий случай все это доступно по ссылке GitHub.

Будем очень признательны за любые ваши комментарии или предложения.

Благодарю.

РЕДАКТИРОВАТЬ: внесены некоторые изменения в код JS, чтобы добавить пояснительные примечания.

1 ответ
1

Плохое название

Первое, что бросилось в глаза, — это название. Начиная с ИМТ

(ИМТ) Индекс массы тела не зависит от времени. Вы показываете рассчитанное значение под заголовком <h1 class="title">Your daily BMI is:</h1> что вообще не имеет смысла.

Я не знаю, какое значение вы рассчитываете. Я предполагаю, что строка «калории в день» найденная в функции revealResult определяет значение, которое вы рассчитываете.

Еще несколько проблем с именами

  • getValues Какие ценности? Может быть, использовать calculateDailyCalories

  • 3 функции с префиксом вычислить? но пользователь дает вам возраст, вес и рост. Вы не рассчитываете эти значения, вы рассчитываете какую-то метрику, исходя из возраста, веса и роста.

    • calculateAge может быть ageMetric
    • calculateWeight может быть weightMetric
    • calculateHeight может быть heightMetric
  • finalResult Отличный финал, но что это? Может быть dailyCalories или просто calories было бы лучше

  • Аргументы в пользу finalResult(x, y, z, gender) ? Это очень плохо, так как не дает никаких указаний на то, что они представляют. Может быть, использовать (ageMetric, weightMetric, heightMetric, isMale)

  • revealResult Немного странно использовать раскрытие, но результат раскрытия не указывает на то, что вы раскрываете. Может быть, использовать displayDailyCalories или displayCalories

Имена переменных должны быть как можно короче. Например, функция calculateHeight(heightFeet, heightInches) Можно сделать вывод, что переменные heightFeet heightInches обозначают высоту, поэтому лучшие названия feet и inches

Имена элементов (идентификаторы) также не указывают на то, что они собой представляют. Смотрите перезапись HTML

Строгий режим

Чтобы раньше выявлять вредные привычки, всегда используйте Strict_mode. Это делается путем добавления директивы use strict в первую строку JS-файла или script тег

ВСЕГДА объявляйте переменные

Если бы вы использовали строгий режим, ваш код выдал бы ошибки для всех необъявленных переменных. Всегда объявляйте переменные с помощью const, где или позволять.

Комментарии

Если вы чувствуете, что вам нужно добавить комментарии для объяснения кода, то сначала подумайте об улучшении наименования и компоновки кода. при комментировании предполагается, что читатель умеет кодировать. Если они не знают, как кодировать, им вообще не следует редактировать ваш код.

Дальнейшие пункты

  • Сохраняйте единый стиль. В некоторых местах вы не добавили точку с запятой в конце строк.

  • Используйте функции, чтобы уменьшить количество повторяющегося кода. (см. перезапись)

  • Избегайте одноразовых переменных.

  • Использовать textContent скорее, чем innerText установить текст элемента.

  • Код, проверяющий пол, очень неясен. Убедитесь, что код не является двусмысленным и вам не нужно искать, что он делает.

CSS

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

document.getElementById('results').style.display = 'none';
// then later in a function
   document.getElementById('results').style.display = 'block'; 

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

/* CSS */
.hidden {display: none}

<!-- HTML -->     
<div id="results" class="hidden">

// JavaScript
document.getElementById('results').classList.remove("hidden")

Не добавляйте код, когда он не нужен

Почему вы добавляете класс calEl.classList.add("finalNumberStyling"); в коде. Элемент скрыт, и вы добавляете его только тогда, когда показываете элемент. Скорее это уже должно быть в HTML

Переписать

Перезапись добавляет несколько вспомогательных функций для удаления подробных вызовов DOM. byId, byName получает элементы по идентификатору и имени. valById получает значение ввода по его идентификатору

Остальная часть перезаписи удаляет много шума кода. Одноразовые переменные, подробные вызовы DOM.

Пол берется прямо из maleRadio (не повторяя переключатели пола) и передается в calories если isMale ложно, то не мужского пола.

Вместо того, чтобы слушать кнопки отправки click событие, когда перезапись прослушивает формы submit мероприятие. Чтобы добавить слушателя, форме был присвоен идентификатор caloriesForm.

Я заменил некоторые входные заголовки тегами меток. Все входы должны использовать label теги, чтобы пометить их, а не просто текстовые элементы рядом с ними.

"use strict";
const byId = id => document.getElementById(id);
const byName = name => document.getElementsByName(name);
const valById = id => parseInt(byId(id).value, 10);

const ageMetric = age => age * 5;
const weightMetric = (stone, lbs) => (stone * 14 + lbs) * 0.453 * 10;
const heightMetric = (feet, inches) => (feet * 12 + inches) * 2.54 * 6.25;

const calories = (ageMetric, weightMetric, heightMetric, isMale) => 
    Math.floor(heightMetric + weightMetric - ageMetric + (isMale ? 5 : -161)); 
    
const displayCalories = calories => {
    byId("resultsContainer").classList.remove("hidden");
    byId("dailyCaloriesDisplay").textContent = calories + ' calories per day'; 
}

byId("caloriesForm").addEventListener('submit', event => {  
    event.preventDefault();
    displayCalories(
        calories(
            ageMetric(valById("ageInput")), 
            weightMetric(valById("stoneInput"), valById("lbsInput")), 
            heightMetric(valById("feetInput"), valById("inchesInput")),
            byId("maleRadio").checked
        )
    );
})
.hidden {display: none}
<form id="caloriesForm">
    <div class="radio-buttons">
        <label for="gender" class="label"><strong>Gender</strong></label >
        <input type="radio" checked="checked" id="maleRadio" name="gender"><label for="male" class="gender-label">Male</label>
        <input type="radio" name="gender"><label for="female" class="gender-label">Female</label>
    </div>

    <label for="ageInput" class="label"><strong>Age</strong></label >
    <input type="number" id="ageInput" class="age-field" min="0" max="130" placeholder="Enter your age"><br>

    <p>Height</p>
    <label for="feetInput" class="label"><strong>Feet</strong></label >
    <input type="number" id="feetInput" class="field" min="0" max="9" placeholder="ft">
    <label for="inchesInput" class="label"><strong>inches</strong></label >
    <input type="number" id="inchesInput" class="field" min="0" max="11"  value="0"><br>

    <p>Weight</p>
    <label for="stoneInput" class="label"><strong>Stone</strong></label >
    <input type="number" id="stoneInput" class="field" min="1" max="80" placeholder="stone">
    <label for="lbsInput" class="label"><strong>lbs</strong></label >
    <input type="number" id="lbsInput" class="field" min="0" max="13" value="0"><br>

    <input type="submit" class="submit">
</form>

<div id="resultsContainer" class="hidden" class="results-container hidden">
    <p id="dailyCaloriesDisplay" class="finalNumberStyling"></p>
</div>

Что еще нужно сделать.

Ваш код не проверяет, была ли форма заполнена. Отображение значения NaN (не числа), если какое-либо из полей не было установлено.

Вы должны определить, есть ли отсутствующие поля, и предложить пользователю добавить недостающую информацию.

Только когда все поля будут введены, вы сможете отобразить результат.

  • Большое спасибо за подробный отзыв. Это было именно то, что мне нужно.

    — Неллингтон

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *