Establecer código de reloj de teoría -- javascript campo con optimization campo con jquery campo con datetime campo con animation camp codereview Relacionados El problema

Set theory clock code


8
vote

problema

Español

Escribí algunos JS para poder alimentar un reloj HTML / CSS "SET TEORY", y me gustaría compartir lo que tengo hasta ahora y preguntar si alguien puede sugerir mejoras.

Específicamente, me pregunto:

  • ¿Debo molestar la comprobación de ver si una variable es mayor que cero antes de ejecutar el bucle?
  • ¿Hay una mejor manera de manejar las clases? Configuración de todos los elementos en "OFF" y luego girar "On", las lámparas correctas parecen una pequeña mano pesada.
  • ¿Debe dividirse la función -- with the same type definitions divisible = ((==) 0 .) . flip mod any' = flip (any . flip id) euler1 = sum . filter $ any' [divisible 3, divisible 5] 3 ?

Para un contexto agregado, puede ver el código en contexto en github y aquí hay una Demo en vivo .

  -- with the same type definitions divisible = ((==) 0 .) . flip mod any' = flip (any . flip id) euler1 = sum . filter $ any' [divisible 3, divisible 5] 4  
Original en ingles

I wrote some JS to power an HTML/CSS "set theory" clock, and I'd like to share what I have so far and ask if anyone can suggest improvements.

Specifically, I'm wondering:

  • Should I bother checking to see if a variable is greater than zero before running the for loop?
  • Is there a better way to handle togging the classes? Setting all elements to "off" and then turning "on" the correct lamps seems a little heavy handed.
  • Should the tick() function be broken up?

For added context, you can view the code in context on GitHub and here is a live demo.

$(document).ready(function () {     startClock(); });  function startClock() {     'use strict';     var delay = 1000;      function tick() {         var clockDate = new Date(),             clockHours = clockDate.getHours(),             clockMinutes = clockDate.getMinutes(),             clockSeconds = clockDate.getSeconds(),             fiveHours = Math.floor(clockHours / 5),             fiveMinutes = Math.floor(clockMinutes / 5),             singleHours = Math.floor(clockHours - (fiveHours * 5)),             singleMinutes = Math.floor(clockMinutes - (fiveMinutes * 5));          $(".circle").toggleClass("off");         $("#display").text(clockHours + ":" + ((clockMinutes < 10) ? "0" + clockMinutes : clockMinutes) + ":" + ((clockSeconds < 10) ? "0" + clockSeconds : clockSeconds));          $("[id^=h]").addClass("off");         $("[id^=m]").addClass("off");          // First row of lamps         if (fiveHours > 0) {             for (var i = 1; i <= fiveHours; i++) {                 $("#h5-" + i).removeClass("off");             }         }          // Second row of lamps         if (singleHours > 0) {             for (var i = 1; i <= singleHours; i++) {                 $("#h1-" + i).removeClass("off");             }         }          // Third row of lamps         if (fiveMinutes > 0) {             for (var i = 1; i <= fiveMinutes; i++) {                 if (i % 3 == 0) {                     $("#m5-" + i).removeClass("off");                 } else {                     $("#m5-" + i).removeClass("off");                 }             }         }          // Fourth row of lamps         if (singleMinutes > 0) {             for (var i = 1; i <= singleMinutes; i++) {                 $("#m1-" + i).removeClass("off");             }         }          setTimeout(tick, delay);     }      tick(); } 
              
 
 

Lista de respuestas

4
 
vote
vote
La mejor respuesta
 

Para responder a sus preguntas por adelantado:

¿Debo molestar la comprobación de ver si una variable es mayor que cero antes de ejecutar el bucle?

No hay necesidad. Lo primero que hace el bucle es la comparación 99887766655443328 . Y dado que function checkContraCode() { //[up, up, down, down, left, right, left, right, b, a, start] var contraCode = [38, 38, 40, 40, 37, 39, 37, 39, 66, 65, 13]; var codeCount = 0; return function(input) { var key = input.which; if (contraCode[codeCount] === key) { codeCount++; //if 11 keys have sequentially matched the cheat code if (codeCount === 11) { alert('You got 30 lives!'); } } else { codeCount = 0; } } } 9 se inicia a 1, no es necesario que se compruebe doblemente diciendo $(window).keydown = checkContraCode(); 0 .

¿Hay una mejor manera de manejar las clases? Configuración de todos los elementos en "OFF" y luego girando "On", las lámparas correctas parecen una pequeña mano pesada.

jQuery's $(window).keydown = checkContraCode(); 1 funciona de unas maneras diferentes . Uno de ellos acepta un booleano como su segundo argumento, que obliga al estado de la palanca: si 99887776655443332 , se agrega el nombre de la clase (si falta), si 998877766554433333 , el nombre de la clase se elimina (si está presente).

Por ejemplo, podría hacer esto:

  $(window).keydown = checkContraCode(); 4  

¿Dónde $(window).keydown = checkContraCode(); 35 es un conjunto de elementos que desea manipular, y 99887776655443336 es el número de lámparas que deben encenderse.

También he gordurado los estados de encendido / apagado, por lo que 99887766555443337 es el nombre de la clase que agrega, mientras que las lámparas de forma predeterminada son OFF . Esto no es realmente importante, pero considerando que llama a los elementos "Lámparas" (aunque indirectamente; están en DIVS clasificados como $(window).keydown = checkContraCode(); 8 S), parece siempre un poco más natural para mí que Ellos por defecto de su estado fuera del estado.

De todos modos, el punto es que lo realizó a través de cada "lámpara", enciéndalo o apagado según sea necesario, en lugar de apagarlos primero.

¿Debería dividirse la función de garrapata ()?

Tienes algo de repetición con los bucles y todo eso, por lo que probablemente sería una buena idea extraer las partes repetidas. Por ejemplo, podría empaquetar el $(window).keydown = checkContraCode(); 9 -loop arriba en una función:

  keyup0  

Luego, simplemente pase un conjunto de elementos, y el "Cuenta de lámpara encendida".

Para esto, también podría valer la pena echar un vistazo al HTML. En este momento, tiene IDS para cada lámpara individual. Pero ese es un poco demasiado específico para mi gusto, ya que todos son esencialmente el mismo tipo de cosas. No hay gran necesidad de darles identificaciones únicas.

En su lugar, lo sugirí en la identificación de las filas, y use un selector como keyup1 (por ejemplo) para encontrar las lámparas, donde 99887776655443342 es el ID de la fila. Esto también le permitirá encontrarlos una vez y simplemente bucle a través de ellos en cada garrapata, en lugar de volver a encontrarlos cada vez.

Por cierto, use un selector de nombre de clase para encontrar el círculo de parpadeo, aunque, en su caso, debe tener una identificación ya que es un elemento único.

Otra alternativa es construir los elementos en el código, e inyectarlos en la página. La ventaja sería que no tenga un acoplamiento ajustado entre el marcado y el JS (si cambia, el otro podría romperse). La desventaja es que ahora tiene un marcado en usted JS (mientras que el CSS todavía está en otro lugar). Si bien le sugiero que lo pruebes, no recomendaré necesariamente un enfoque sobre el otro, cuando sea solo una sola página de página / una sola función. (Si en su lugar está pensado como un widget quería agregarse a cualquier tipo de página, es una historia diferente).

Otras cosas:

  • en lugar de keyup3 Puede simplemente usar el operador de módulo para obtener el resto de una división: keyup4 . Lo mismo para keyup5 Por supuesto.

  • La línea utilizada para actualizar el elemento keyup6 es extremadamente largo y repetitivo. Considere hacer una función keyup77 para manejar el levantamiento pesado.

  • Tiene clases como keyup8 , keyup9 99887766555443350 que describe Apariencia mucho para específicamente. ¿Qué pasa si decides las horas deben ser triángulos púrpuras? Las clases que tienen ahora son repentinamente engañosas. Siempre apunte a los nombres de las clases, describa lo que es algo es , no cómo se ve .

  • Hablando de semántica, podría considerar el uso de elementos de lista keydown1 para las lámparas, ya que son esencialmente ordenadas listas de elementos.

  • El keydown2 se puede reducir simplemente keydown3

  • Por último, podría considerar saltarse algunos pasos (bucles) por completo. Por ejemplo, solo si keydown4 es necesario actualizar "FIV minuto "Lámparas; solo si keydown5 es cero, ¿necesita actualizar las lámparas" de una sola hora ", etc. En otras palabras, solo necesita actualizar los pasos más" gruesos ", si el Los granos finos se envuelven.

Aquí hay un fragmento que incorpora algunos de los comentarios anteriores.

  keydown6  
  keydown7  
  keydown8  

 

To answer your questions up front:

Should I bother checking to see if a variable is greater than zero before running the for loop?

No need. The first thing the loop does is the i <= x comparison. And since i starts at 1, there's no need for you to double-check by saying if(x > 0).

Is there a better way to handle togging the classes? Setting all elements to "off" and then turning "on" the correct lamps seems a little heavy handed.

jQuery's toggleClass() works in a few different ways. One of them accepts a boolean as its 2nd argument, which forces the toggle state: If true, the class name is added (if missing), if false, the class name is removed (if present).

So for instance, you might do this:

lamps.each(function (index) {   $(this).toggleClass("on", index < count); }); 

Where lamps is a set of elements you want to manipulate, and count is the number of lamps that should light up.

I've also flipped the on/off states, so on is the class name you add, while the lamps by default are off. This isn't really important, but considering you call the elements "lamps" (albeit indirectly; they're in divs classed as lamp-rows), it seems ever-so-slightly more natural to me that they default to their off state.

Anyway, the point is that you loop through each "lamp", turning it on or off as needed, instead of turning them all off first.

Should the tick() function be broken up?

You've got some repetition with the loops and all that, so it'd probably be a good idea to extract the repeated parts. For instance, you could package the each-loop above in a function:

 function updateLamps(lamps, count) { ... } 

Then simply pass it a set of elements, and the "lit lamp count".

For this, it might also be worth taking a look at the HTML. Right now, you have IDs for each individual lamp. But that's a tad too specific for my liking, since they're all essentially the same kind of thing. No great need to give them unique IDs.

I'd suggesting IDing the rows instead, and use a selector like #single-hours > .lamp (for instance) to find the lamps, where #single-hours is the ID of the row. This will also allow you to find them once and simply loop through them on each tick, rather than re-finding them each time.

Incidentally, you use a class name selector to find the blinking circle, although that, if anything, should have an ID as it's a unique element.

Another alternative is to build the elements in code, and inject them into the page. The advantage would be that you don't have tight coupling between the markup and the JS (if either changes, the other might well break). The disadvantage is that you now have markup in you JS (while the CSS is still somewhere else). While I'd suggest you try it out, I won't necessarily recommend one approach over the other, when it's just a single page/single feature thing. (If it's instead intended as a widget meant to be added to any kind of page, it's a different story).

Other things:

  • Instead of Math.floor(clockMinutes - (fiveMinutes * 5)) you can simply use the modulo operator to get the remainder of a division: clockMinutes % 5. Same for singleHours of course.

  • The line used to update the #display element is exceedingly long and repetitive. Consider making a formatTime function (or similar) to handle the heavy-lifting.

  • You have classes like rectangle-*, yellow and red which describe appearance much to specifically. What if you decide the hours should be purple triangles? The classes they have now are suddenly misleading. Always aim for class names the describe what something is, not how it looks.

  • Speaking of semantics, you could consider using ol list elements for the lamps, since they are essentially ordered lists of elements.

  • The whole $(document).ready(...) thing can be shortened to simply $(startClock);

  • Lastly, you could consider skipping some steps (loops) entirely. For instance, only if singleMinutes == 0 is there a need to update "five minute" lamps; only if fiveMinutes is zero, do you need to update the "single hour" lamps, etc.. In other words, you only need to update the more "coarse" steps, if the finer-grained ones roll over.

Here's a snippet that incorporates some of the comments above.

function startClock() {     'use strict';     var delay = 1000,         display = $("#display"),         secondsLamp = $("#seconds-lamp"),         fiveHourLamps   = $("#five-hour-lamps > div"),         oneHourLamps    = $("#one-hour-lamps > div"),         fiveMinuteLamps = $("#five-minute-lamps > div"),         oneMinuteLamps  = $("#one-minute-lamps > div");      function toggleLamps(lamps, count) {       lamps.each(function (index) {         $(this).toggleClass('off', index >= count);       });     }      function tick() {         var clockDate = new Date(),             clockHours = clockDate.getHours(),             clockMinutes = clockDate.getMinutes(),             clockSeconds = clockDate.getSeconds(),             fiveHours = Math.floor(clockHours / 5),             fiveMinutes = Math.floor(clockMinutes / 5),             singleHours = clockHours % 5,             singleMinutes = clockMinutes % 5;          secondsLamp.toggleClass("off");         display.text(clockHours + ":" + ((clockMinutes < 10) ? "0" + clockMinutes : clockMinutes) + ":" + ((clockSeconds < 10) ? "0" + clockSeconds : clockSeconds));          toggleLamps(fiveHourLamps, fiveHours);         toggleLamps(oneHourLamps, singleHours);         toggleLamps(fiveMinuteLamps, fiveMinutes);         toggleLamps(oneMinuteLamps, singleMinutes);          setTimeout(tick, delay);     }      tick(); }  $(startClock);
body {     background: rgba(0,0,0,0.35); }  .clock {     width: 16rem;     margin: 3em auto 0 auto;     font-family: sans-serif; } .circle {     width: 5rem;     height: 5rem;     border-radius: 50%;     border: 1rem solid rgba(204, 204, 204, 1);     margin: 0 auto 0 auto; } .lamp-row {     border-radius: 1rem;     background: rgba(204, 204, 204, 1);     display: flex;     padding: 0.5rem;     margin: 0.5rem 0 0 0; } .rectangle-large, .rectangle-small {     border-radius: 0.5rem;     height: 2.5rem;     box-sizing:border-box;     -moz-box-sizing:border-box;     -webkit-box-sizing:border-box;     margin: 0.1rem;     display: inline-block; } .rectangle-large {     width: 4rem; } .rectangle-small {     width: 1.45rem; } .red {     background: rgba(255, 0, 0, 1); } .red.off {     background: rgba(255, 0, 0, 0.5); } .yellow {     background: rgba(255, 255, 0, 1); } .yellow.off {     background: rgba(255, 255, 0, 0.4); } #display {     margin-top: 1rem;     text-align: center; }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script> <div class="clock">     <div>         <div id="seconds-lamp" class="circle yellow"></div>     </div>     <div id="five-hour-lamps" class="lamp-row">         <div class="rectangle-large red"></div>         <div class="rectangle-large red"></div>         <div class="rectangle-large red"></div>         <div class="rectangle-large red"></div>     </div>     <div id="one-hour-lamps" class="lamp-row">         <div class="rectangle-large red"></div>         <div class="rectangle-large red"></div>         <div class="rectangle-large red"></div>         <div class="rectangle-large red"></div>     </div>     <div id="five-minute-lamps" class="lamp-row">         <div class="rectangle-small yellow"></div>         <div class="rectangle-small yellow"></div>         <div class="rectangle-small red"></div>         <div class="rectangle-small yellow"></div>         <div class="rectangle-small yellow"></div>         <div class="rectangle-small red"></div>         <div class="rectangle-small yellow"></div>         <div class="rectangle-small yellow"></div>         <div class="rectangle-small red"></div>         <div class="rectangle-small yellow"></div>         <div class="rectangle-small yellow"></div>     </div>     <div id="one-minute-lamps" class="lamp-row">         <div class="rectangle-large yellow"></div>         <div class="rectangle-large yellow"></div>         <div class="rectangle-large yellow"></div>         <div class="rectangle-large yellow"></div>     </div>     <div>         <div id="display"></div>     </div> </div>
 
 
 
 
3
 
vote

La demostración en vivo se ve muy bien :)

Qué pasa con esto:

              if (i % 3 == 0) {                 $("#m5-" + i).removeClass("off");             } else {                 $("#m5-" + i).removeClass("off");             }   

parece una declaración sin sentido if con una sola línea:

              $("#m5-" + i).removeClass("off");   

Esta línea muy larga plantea alguna generalización:

      $("#display").text(clockHours + ":" + ((clockMinutes < 10) ? "0" + clockMinutes : clockMinutes) + ":" + ((clockSeconds < 10) ? "0" + clockSeconds : clockSeconds));   

Por ejemplo, podría crear una función de ayuda para reducir la duplicación de la lógica y hacer que el código sea más legible:

  function format(num) { return num < 10 ? '0' + num : num; } $("#display").text(clockHours + ":" + format(clockMinutes) + ":" + format(clockSeconds));   
 

The live demo looks very nice :)

What's up with this though:

            if (i % 3 == 0) {                 $("#m5-" + i).removeClass("off");             } else {                 $("#m5-" + i).removeClass("off");             } 

Looks like a pointless if statement, that can be replaced with a single line:

            $("#m5-" + i).removeClass("off"); 

This very long line begs for some generalization:

    $("#display").text(clockHours + ":" + ((clockMinutes < 10) ? "0" + clockMinutes : clockMinutes) + ":" + ((clockSeconds < 10) ? "0" + clockSeconds : clockSeconds)); 

For example you could create a helper function to reduce duplication of logic and make the code more readable:

function format(num) { return num < 10 ? '0' + num : num; } $("#display").text(clockHours + ":" + format(clockMinutes) + ":" + format(clockSeconds)); 
 
 
 
 

Relacionados problema

4  Carrusel para sitio web  ( Carousel for website ) 
He estado construyendo mi propio carrusel en los últimos días. No soy un guru jquery, solo un entusiasta, y creo que mi código es un poco descuidado, de ahí l...

3  Forma aceptable de usar métodos jQuery en directivas angulares  ( Acceptable way of using jquery methods in angular directives ) 
aquí es mi ejemplo de plunker de lo que estoy haciendo. jQuery es una excelente manera de usar transiciones de diapositivas y parece funcionar bien con an...

2  Los efectos de desplazamiento, fade-in y fade-out se sienten clunky  ( Scrolling fade in and fade out effects feeling clunky ) 
Estoy cerca de terminar un sitio donde hay una serie de funciones de jQuery que están pasando. Todos trabajan, pero la página está corriendo un poco demasiado...

1  Animación "Star Field" con JavaScript y Sass-CSS  ( Star field animation with javascript and sass css ) 
He hecho esta animación el fin de semana como una cosa "solo por diversión". Y para jugar con varias técnicas que he visto en otros Código. Creo que funcion...

7  Una secuencia parcialmente superpuesta de animaciones jQuery  ( A partially overlapping sequence of jquery animations ) 
¡Sé que este es un desastre! ¿Cómo puedo escribir este código mejor? function anim() { $( "#p1-animation-1" ).fadeIn( 1200, function() { ...

2  Animaciones de la línea de tiempo  ( Timeline animations ) 
Hice recientemente esta línea de tiempo del campus para mi universidad. Al ver la línea de tiempo en un dispositivo móvil (no una tableta), la barra de nave...

1  Manipulador de eventos repetitivos para un control de interfaz de usuario de la UI  ( Repetitive event handler for a toggling ui control ) 
Siento que este tipo de código podría haber sido escrito más elegante, especialmente con las enormes afirmaciones de IF / ODS. ¿Alguien puede ayudarme a rompe...

2  Animar una caja, rebotando a la izquierda y la derecha  ( Animate a box bouncing left and right ) 
Escribí el siguiente código para mover un cuadro a la derecha y devolverlo a la izquierda, y repetir esta acción para siempre. Este código funciona, pero ¿h...

3  Cajas de texto ampliables para leyendas de campos  ( Expandable text boxes for legends of fieldsets ) 
¿Cuál es la mejor manera de refactorizar el siguiente script? <?php // First we execute our common code to connection to the database and start the s...

12  Impresión similar a la consola del mensaje JavaScript  ( Console like printing of message javascript ) 
Estoy aprendiendo a Javascript y he hecho un pequeño script de modificación de documentos muy simple que imprime un mensaje como si fuera alguien escribiendo ...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos