Снегопад в HTML

Я новичок, и я сделал Снегопад в HTML для моей мамы. Я уверен, что это не будет выглядеть потрясающе для любого разработчика там, но, эй, вот почему я разместил его.

Мне хотелось бы получить общий обзор этого. Я особенно обеспокоен качеством и усовершенствованиями этого HTML.

<h1>Happy Winter!</h1><script>

var snowmax=35
var snowcolor=new Array("#aaaacc","#ddddFF","#ccccDD")
var snowtype=new Array("Arial Black","Arial Narrow","Times","Comic Sans MS")
var snowletter="*"
var sinkspeed=0.6
var snowmaxsize=22
var snowminsize=8
var snowingzone=3

var snow=new Array()
var marginbottom
var marginright
var timer
var i_snow=0
var x_mv=new Array();
var crds=new Array();
var lftrght=new Array();
var browserinfos=navigator.userAgent 
var ie5=document.all&&document.getElementById&&!browserinfos.match(/Opera/)
var ns6=document.getElementById&&!document.all
var opera=browserinfos.match(/Opera/)  
var browserok=ie5||ns6||opera

function randommaker(range) {       
    rand=Math.floor(range*Math.random())
    return rand
}

function initsnow() {
    if (ie5 || opera) {
        marginbottom = document.body.clientHeight
        marginright = document.body.clientWidth
    }
    else if (ns6) {
        marginbottom = window.innerHeight
        marginright = window.innerWidth
    }
    var snowsizerange=snowmaxsize-snowminsize
    for (i=0;i<=snowmax;i++) {
        crds[i] = 0;                      
        lftrght[i] = Math.random()*15;         
        x_mv[i] = 0.03 + Math.random()/10;
        snow[i]=document.getElementById("s"+i)
        snow[i].style.fontFamily=snowtype[randommaker(snowtype.length)]
        snow[i].size=randommaker(snowsizerange)+snowminsize
        snow[i].style.fontSize=snow[i].size
        snow[i].style.color=snowcolor[randommaker(snowcolor.length)]
        snow[i].sink=sinkspeed*snow[i].size/5
        if (snowingzone==1) {snow[i].posx=randommaker(marginright-snow[i].size)}
        if (snowingzone==2) {snow[i].posx=randommaker(marginright/2-snow[i].size)}
        if (snowingzone==3) {snow[i].posx=randommaker(marginright/2-snow[i].size)+marginright/4}
        if (snowingzone==4) {snow[i].posx=randommaker(marginright/2-snow[i].size)+marginright/2}
        snow[i].posy=randommaker(2*marginbottom-marginbottom-2*snow[i].size)
        snow[i].style.left=snow[i].posx
        snow[i].style.top=snow[i].posy
    }
    movesnow()
}

function movesnow() {
    for (i=0;i<=snowmax;i++) {
        crds[i] += x_mv[i];
        snow[i].posy+=snow[i].sink
        snow[i].style.left=snow[i].posx+lftrght[i]*Math.sin(crds[i]);
        snow[i].style.top=snow[i].posy

        if (snow[i].posy>=marginbottom-2*snow[i].size || parseInt(snow[i].style.left)>(marginright-3*lftrght[i])){
            if (snowingzone==1) {snow[i].posx=randommaker(marginright-snow[i].size)}
            if (snowingzone==2) {snow[i].posx=randommaker(marginright/2-snow[i].size)}
            if (snowingzone==3) {snow[i].posx=randommaker(marginright/2-snow[i].size)+marginright/4}
            if (snowingzone==4) {snow[i].posx=randommaker(marginright/2-snow[i].size)+marginright/2}
            snow[i].posy=0
        }
    }
    var timer=setTimeout("movesnow()",50)
}

for (i=0;i<=snowmax;i++) {
    document.write("<span id='s"+i+"' style='position:absolute;top:-"+snowmaxsize+"'>"+snowletter+"</span>")
}
if (browserok) {
    window.onload=initsnow
}
29 голосов | спросил Ducky 6 J0000006Europe/Moscow 2014, 13:39:00

4 ответа


23

Некоторые предложения:

  1. Использование большего количества пробелов и точек с запятой
  2. Инициализировать массивы с помощью []

    var snowcolor = ["#aaaacc", "#ddddFF", "#ccccDD"];
    
  3. Используйте соглашения об именах для имен переменных и методов, чтобы сделать их более читаемыми

    initSnow() or init_snow()
    browserInfos or browser_infos
    
  4. В JavaScript рекомендуется объединить несколько переменных declerations. Поэтому вместо

    var marginbottom
    var marginright
    var timer
    

    вы можете написать

    var marginbottom,
        marginright,
        timer;
    
  5. Вместо передачи строки в setTimeout вы можете напрямую передать ссылку на функцию

    var timer = setTimeout(movesnow, 50);
    
  6. В вашем коде есть несколько магических чисел, которые вы можете захотеть извлечь из переменных.

  7. Так как JavaScript не имеет final static переменных (afaik), рассмотрите их маркировку с помощью соглашений об именах. Например:

    var SNOW_MAX_SIZE = 22;
    
  8. Четыре оператора if внутри initsnow и movesnow кажутся одинаковыми, поэтому вы можете переместить их в отдельную функцию. Однако в настоящее время у вас snowingzone установлено фиксированное значение, и вы его не меняете. Таким образом, операторы if не нужны.

  9. Некоторые имена переменных довольно загадочны: x_mv, lftrght и т. д.

  10. Существуют неиспользуемые переменные: например. i_snow

  11. Идентификация браузера и «расчет» кода browserok кажутся хорошими кандидатами для отдельной функции.


Обновить Добавлено 8. - 11.

Если вы чувствуете себя уверенно или хотите попробовать статический анализ кода, вам может понадобиться посмотреть что-то вроде http://www.jslint.com / или http://www.jshint.com/. Они могут предупреждать вас о глобальных переменных, неиспользуемых переменных и т. Д.

ad 4) Как упоминалось в Schism, эти советы могут привести к нежелательным глобальным переменным, если вы перепутаете , и ;. Так, например:

function foo() {
    var a,
        b; // Oops! ; should be ,
        c; // c is now a global variable
}
ответил Syjin 6 J0000006Europe/Moscow 2014, 15:38:29
17

Вместо перезапуска setTimeout() при каждом вызове movesnow(), я предлагаю позвонить setInterval() только один раз для всей программы.

У вас есть дублированный код между initsnow() и movesnow() (переключатель snowingzone), который следует учитывать. Похоже, что snowingzone жестко закодирован до 3, хотя, поэтому я не уверен, каково ваше реальное намерение.

Считается хорошей практикой завершать каждый оператор явной точкой с запятой, даже если JavaScript не требует этого.

Я предлагаю переименовать массив snow в flakes и, возможно, переименовать несколько других переменных таким же образом. Я не уверен, что означает переменная crds - аббревиатура слишком загадочна для меня.

ответил 200_success 6 J0000006Europe/Moscow 2014, 14:05:12
7

Я не делал веб-разработчиков в течение очень долгого времени, поэтому я оставлю ваш фактический вопрос экспертам. Я просто хочу указать небольшой стиль. Назначения переменных могут использовать некоторое пространство для передышки.

var crds = new Array();

То же самое в ваших циклах.

for (i = 0; i <= snowmax; i++) {
ответил RubberDuck 6 J0000006Europe/Moscow 2014, 14:16:13
7

В дополнение к ответам выше (с которыми я полностью согласен), два бита от меня:

  1. Используйте оператор switch и /или факторинг смарт-кода, чтобы

    if (snowingzone==1) {snow[i].posx=randommaker(marginright-snow[i].size)}
    if (snowingzone==2) {snow[i].posx=randommaker(marginright/2-snow[i].size)}
    if (snowingzone==3) {snow[i].posx=randommaker(marginright/2-snow[i].size)+marginright/4}
    if (snowingzone==4) {snow[i].posx=randommaker(marginright/2-snow[i].size)+marginright/2}
    

    изменяется на

    switch( snowingzone ) {
        case 1:
            snow[i].posx = randommaker( marginright - snow[i].size ); break;
        case 2:
            snow[i].posx = randommaker( marginright / 2-snow[i].size ); break;
        case 3:
            snow[i].posx = randommaker( marginright / 2-snow[i].size ) + marginright / 4; break;
        case 4:
            snow[i].posx = randommaker( marginright / 2-snow[i].size ) + marginright / 2; // break; is optional here, you may use it for clarity
    }
    

    или, еще более короче, до

    snow[i].posx = randommaker( marginright / ( snowingzone == 1 ) ? 1 : 2 - snow[i].size);
    if ( snowingzone == 3 )
        snow[i].posx += marginright / 4;
    else if ( snowingzone == 4 )
        snow[i].posx += marginright / 2;
    

    https://developer.mozilla.org/EN-US /Docs /Web /JavaScript /Справка /Заявления /выключатель

  2. Не создавайте элементы DOM с помощью:

    document.write("&lt;span id='s"+i+"' style='position:absolute;top:-"+snowmaxsize+"'>"+snowletter+"&lt;/span>");
    

    Вместо этого вы должны использовать:

    snow[i] = document.createElement("span"); // etc
    

    Это позволяет вам создать массив чешуек непосредственно и полностью пропускать вызов snow[i]=document.getElementById("s"+i).

    https://developer.mozilla.org/en- США /документы /Web /API /document.createElement

ответил vaxquis 8 J0000006Europe/Moscow 2014, 17:01:10

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132