2016-09-26 2 views
2

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

var time = new Date().getHours(); 
var greet; 

if (time >= 0 && time < 4) { 

    greet = 'Wow you\'re up late! Good morning.'; 
} 

else if (time >= 4 && time < 12) { 

    greet = 'Good morning.'; 
} 

else if (time >= 12 && time < 17) { 

    greet = 'Good afternoon.'; 
} 

else if (time >= 17 && time <= 23) { 

    greet = 'Good evening.'; 
} 

document.querySelector('.js-greet').innerHTML = greet; 

Любое понимание было бы весьма полезным.

ответ

2

Другой альтернативой

let time = new Date().getHours(); 

let hours = [4, 12, 17, 24]; 
let greetings = ['Wow you\'re up late! Good morning.', 'Good morning.', 'Good afternoon.', 'Good evening.']; 

console.log(greetings[hours.findIndex(hour => hour > time)]); 
+1

Мне это нравится (ES6) тоже! – trincot

+0

Это именно тот тип ответа, на который я надеялся, он достаточно читабельный, он современный, он аккуратный и аккуратный. Я буду использовать это решение и polyfill 'findIndex' для тех бедных душ в IE11 https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex – matt3224

+0

@ matt3224, вы можете делать то, что тебе нравится. ИМО, это жертвует удобочитаемостью, ремонтопригодностью, совместимостью между браузерами и, возможно, скоростью для «отличной». Теперь, если вы инкапсулировали это так, что это было фактически ООП, или, по крайней мере, использовали один объект с свойствами «startTime» и «message», а не двумя отдельными (глобальными) массивами (которые не имеют неотъемлемых отношений), это могут иметь некоторые функции погашения. Для того, чтобы делать то, что вы на самом деле делаете, это плохой выбор. [Это не плохой пример того, что можно сделать, это просто нехороший выбор того, что нужно сделать.] – Makyen

1

Во-первых, вам не нужны нижние границы, потому что getHours никогда не вернется < 0 и на else if с, вы не получите там, если раньше граница соответствует; и вам не нужны какие-либо условия на последней вообще, так как первый свинг: (. Также удален, что казалось мне ненужный вертикальным пробел)

var time = new Date().getHours(); 
var greet; 

if (time < 4) { 
    greet = 'Wow you\'re up late! Good morning.'; 
} 
else if (time < 12) { 
    greet = 'Good morning.'; 
} 
else if (time < 17) { 
    greet = 'Good afternoon.'; 
} 
else { 
    greet = 'Good evening.'; 
} 

document.querySelector('.js-greet').innerHTML = greet; 

Кроме что вы начинаете входить в карты сообщений, по этим будет список определенных часов, а не диапазонов, поэтому, вероятно, это достаточно хорошо.

1

Ну, вы можете упростить свои заявления if из-за того, что уже проверили (в более ранних операциях if) для значений, которые меньше желаемого диапазона. Кроме того, getHours() возвращает целое число от 0 до 23, поэтому нет причин проверять номера за пределами этих границ.

var time = new Date().getHours(); 
 
var greet; 
 

 
if (time < 4) { 
 
    greet = "Wow you're up late! Good morning."; 
 
} else if (time < 12) {  //You can't get here if time < 4; no need to check again. 
 
    greet = 'Good morning.'; 
 
} else if (time < 17) {  //You can't get here if time < 12; no need to check again. 
 
    greet = 'Good afternoon.'; 
 
} else {      //You can't get here if time < 17; no need to check again. 
 
    greet = 'Good evening.'; 
 
} 
 

 
document.querySelector('.js-greet').innerHTML = greet;
<div class="js-greet"></div>

Кроме того, вы могли бы устранить окончательный else путем присвоения 'Good evening.' к greet в качестве начального значения.

var time = new Date().getHours(); 
 

 
var greet = 'Good evening.'; //Remains the value for greet for time >=17 
 
if (time < 4) { 
 
    greet = "Wow you're up late! Good morning."; 
 
} else if (time < 12) { 
 
    greet = 'Good morning.'; 
 
} else if (time < 17) { 
 
    greet = 'Good afternoon.'; 
 
} 
 

 
document.querySelector('.js-greet').innerHTML = greet;
<div class="js-greet"></div>

+0

Только то, о чем я думал. :) +! –

+0

Эй, Майкен, я открыт для того, что вы считаете лучшим решением. Я прав, думая, что это так? – matt3224

+0

@ matt3224, сложнее, чем выше (или фактически идентичный ответ T.J. Crowder) зависит от того, что вы собираетесь использовать для него (и если то, что вы запрашивали, было просто примером или фактическим использованием). Если это просто одноразовый, который никогда не будет жить в Интернете, то все, что вам нравится в браузере, которое вы собираетесь использовать, в порядке. Если это то, что входит в производственный код, тогда возникают другие соображения. В общем, наличие строк в структуре данных - это хорошо (приближается к возможности интернационализации). (продолжение) – Makyen

2

Вы могли бы - в качестве альтернативы - определить массив со значением в час. Каждое значение будет идентификатором отображаемого сообщения. Этот массив может быть даже строка, а строки также позволяют получить доступ каждый символ с помощью нотации массива:

var time = new Date().getHours(); 
 

 
var greet = ['Wow you\'re up late! Good morning.', 
 
      'Good morning.', 
 
      'Good afternoon.', 
 
      'Good evening.']['000011111111222223333333'[time]]; 
 

 
console.log(greet);

+0

Хорошее использование строки в качестве таблицы поиска – Redu

+0

Это аккуратно, но не очень читаемо или разумно для других. – Shadetheartist

+1

Несмотря на то, что это интересная/поучительная альтернатива, это значительно повышает читаемость/ремонтопригодность, помещая часы, в которые приветствие переходит в строку '000011111111222223333333''. Чтобы понять, что предназначено, или, по крайней мере, когда в течение дня отображается каждое приветствие, человек, читающий код, должен вручную * подсчитать * символы в этой строке. – Makyen

0

Хорошо на вас за вопрос, как улучшить! Я вижу такой код слишком часто, и есть много способов его упростить.

Ваш оригинальный код

if (time >= 0 && time < 4) { 

    greet = 'Wow you\'re up late! Good morning.'; 
} 

else if (time >= 4 && time < 12) { 

    greet = 'Good morning.'; 
} 

else if (time >= 12 && time < 17) { 

    greet = 'Good afternoon.'; 
} 

else if (time >= 17 && time <= 23) { 

    greet = 'Good evening.'; 
} 

Улучшение # 1

Здесь мы несколько первых шагов и удаления избыточного кода/упростить вещи

if(time < 0) { 
    //dont do anything 
} 
// now we can take "time >= 0 && " out, making it easier to read 
else if (time < 4) { 

    greet = 'Wow you\'re up late! Good morning.'; 
} 
//here, we don't actually need to check if time >= 4, because if it wasn't 
//it would have been dealt with by a previous condition 
else if (time < 12) { 

    greet = 'Good morning.'; 
} 
//and the same for the rest of them 
else if (time < 17) { 

    greet = 'Good afternoon.'; 
} 

//lets keep things consistant by using "time < 24" rather than "time <= 23" 
else if (time < 24) { 

    greet = 'Good evening.'; 
} 

Улучшение # 2

Перепроверка структурирование может упростить код еще больше, так что позволяет бросить этот код в функцию

//in this function we can take advantage of the order of operations 
//to simplify further 
function getGreeting(time){ 

    //returning early when things go wrong is always a good idea 
    if(time < 0) { 
     return null; 
    } 

    //We dont need 'else' because we are returning when a condition is met, 
    //and the app doesn't have a chance to try the other conditionals 
    //the less 'else's you have the better the code IMO :) 
    if (time < 4) { 

     //return our string instead of setting it! 
     return 'Wow you\'re up late! Good morning.'; 
    } 

    if (time < 12) { 
     return 'Good morning.'; 
    } 

    if (time < 17) { 
     return 'Good afternoon.'; 
    } 

    if (time < 24) { 
     return 'Good evening.'; 
    } 

} 

//then call the function 
greet = getGreeting(time); 

Улучшение # 3

Мы можем сделать лучше ... со структурами данных!

function getGreeting(time){ 

    //we still check for stuff that shouldn't happen 
    if(time < 0) { 
     return null; 
    } 

    //Here we declare a 'greetings' array. In the array there are objects! 
    //each object has a field for time start and end as well as a greeting 
    //the thing that makes this approach so neat is that adding more data in 
    //the future 
    //is dead easy, and the data can now come from a database or whatever 
    //you want. 
    var greetings = [ 
     { 
      timeStart: 0, 
      timeEnd: 4, 
      greeting: 'Wow you\'re up late! Good morning.' 
     }, 
     { 
      timeStart: 4, 
      timeEnd: 12, 
      greeting: 'Good morning.' 
     }, 
     { 
      timeStart: 12, 
      timeEnd: 17, 
      greeting: 'Good afternoon.' 
     }, 
     { 
      timeStart: 17, 
      timeEnd: 24, 
      greeting: 'Good evening.' 
     } 
    ]; 

    //We can add as many new items to the greetings array without ever 
    // touching this again! 
    for(var i = 0; i < greetings.length; i++) 
    { 
     if(time >= greetings[i].timeStart && time < greetings[i].timeEnd) 
     { 
      return greetings[i].greeting; 
     } 
    } 

} 

//then call the function 
greet = getGreeting(time); 
+0

Если у вас есть что-то вроде этого, пожалуйста, вручную оберните свои комментарии на новую строку, чтобы нам не нужно прокручивать по горизонтали, просто чтобы прочитать ваши комментарии. Кроме того, учитывая, что вы представляете несколько улучшений (либо поэтапно, либо как альтернативы), было бы лучше, если бы вы представили различные улучшения в отдельных блоках кода, чтобы разделение было более ясным. Это также дает вам возможность обсуждать, почему вы предлагаете дополнительные/альтернативные улучшения в регулярном тексте между блоками кода. – Makyen

+0

@may все хорошие моменты, отредактируйте – Shadetheartist

Смежные вопросы