Как упростить js код?

Question

[4uva4ok1905]

Как можно упростить код, суть: есть фотографии которые берутся с папки(1..9) случайным образом, JSON файл и 4 кнопки с вариантами ответов

function load() {
  window.platform = "http://first/paint/" 
  window.basicSet = [1,2,3,4,5,6,7,8,9];
  window.currentSet = basicSet;
  getart();
};

load();

function getart() { 
  currentWins();
  var art = document.getElementById("art");
  window.truePainter = window.currentSet[Math.floor((Math.random()*window.currentSet.length))];
  $.getJSON("painters/" + window.truePainter + "/data.json", function(json) {
     ...
      window.truePainterName = i18n.t("painters." + truePainter, { lng: window.lang });
      putButtons(window.truePainterName);
  });
};

function putButtons(painter) {
  function randomPainter() {
    var random = "painters." + Math.floor((Math.random()*9)+1)
    return i18n.t(random, { lng: window.lang });
  };
  
  var painters = [painter];
  for (var i=0;i<10;i++) {
    painters.push(randomPainter());
    if (painters[1] == "") {
      location.reload(); 
    };
  };
   
  painters = painters.reverse().filter(function (e, i, painters) {
     return painters.indexOf(e, i+1) === -1;
  }).reverse(); 

  var buttons = [];
  buttons.push(painters[0]);      
  buttons.push(painters[1]);
  buttons.push(painters[2]);
  buttons.push(painters[3]);     
  
  function shuffle(o){
    for(var j, x, i = o.length; i; j = Math.floor(Math.random() * i), x = o[--i], o[i] = o[j], o[j] = x);
    return o;
  };

  shuffle(buttons);
  document.getElementById("btn1").innerHTML = buttons[0];
  document.getElementById("btn2").innerHTML = buttons[1];
  document.getElementById("btn3").innerHTML = buttons[2];
  document.getElementById("btn4").innerHTML = buttons[3];
};

function checkAnswer(btn) { 
var answer = document.getElementById(btn).innerHTML;

if (answer == window.truePainterName) {
...

Answer 1

[Владлен Грачев]

Выкинуть глобальные переменные.

Что касается имен: i, j, x — нормальная практика, хотя обычно с i и j идет какая-нибудь k. lng — тоже все ясно. Вот с i18n.t явно надо что-то делать, ибо она берется откуда-то извне, непонятно, что делает и к чему относится. Можно, конечно, догадаться, что это объект интернационализации, возвращающий заданное значение на текущем языке, но все же. Если не оформлять все это в объект отдельный, то хотя бы в качестве параметров функций такие вещи передавать стоит, может, читабельнее станет код.

А вот сам по себе цикл в shuffle() читается ужасно. И не в именовании дело, а в оформлении.

buttons.push(painters[0]);      
buttons.push(painters[1]);
buttons.push(painters[2]);
buttons.push(painters[3]);
...
document.getElementById("btn1").innerHTML = buttons[0];
document.getElementById("btn2").innerHTML = buttons[1];
document.getElementById("btn3").innerHTML = buttons[2];
document.getElementById("btn4").innerHTML = buttons[3];

Тут проще обойтись циклом или forEach(). Если filter() используете, видимо, предполагается его поддержка, а значит и forEach(). Для первой части можно использовать и slice().

Answer 2

[vasIvas]

Я бы посоветовал "усложнить код" нормальными названиями!
i18n.t, lng, j, x, i - у Вас что, ревматизм? Напишите полные названия,
а то не хочется настолько всматриваться, чтобы понять что Вы в цикле делаете.

Comments

[SlivTime]

Цикл, конечно, жесть, но i18n - вполне устоявшееся сокращение.

Answer 3

[Вячеслав]

1. Куча переменных/имен в глобальной области видимости (window) - ужасная идея.
2. Обобщите все это в нормальный объект/несколько объектов, и ваш код станет чище и проще.
Про читабельность и названия вам уже сказали.

Answer 4

[Денис Богданов]

www.ozon.ru/context/detail/id/6287517
JavaScript. Шаблоны - Стоян Стефанов

В этой книге описано как нужно писать код. Написано очень хорошим языком.