Optimizar el bucle JavaScript -- javascript campo con jquery camp codereview Relacionados El problema

Optimise JavaScript loop


1
vote

problema

Español

Tengo este código:

  #ifndef PNG_HH #define PNG_HH #include <png.h>  namespace png {     void write_image(char const *filename, std::uint8_t const *image_data, std::uint32_t image_width, std::uint32_t image_height)     {         /* create a zeroed out png_image struct */         png_image output_png;         std::memset(&output_png, 0, sizeof(output_png));         output_png.version = PNG_IMAGE_VERSION;         output_png.format = PNG_FORMAT_GRAY;         output_png.width = image_width;         output_png.height = image_height;          /* write the png file */         png_image_write_to_file(&output_png, filename, 0, image_data, image_height, nullptr);          /* cleanup */         png_image_free(&output_png);     } } #endif 7  

El archivo zgcontacts.json se puede encontrar aquí .

Cualquier puntero para optimizar esto es muy apreciado. Por ejemplo, este bucle:

  #ifndef PNG_HH #define PNG_HH #include <png.h>  namespace png {     void write_image(char const *filename, std::uint8_t const *image_data, std::uint32_t image_width, std::uint32_t image_height)     {         /* create a zeroed out png_image struct */         png_image output_png;         std::memset(&output_png, 0, sizeof(output_png));         output_png.version = PNG_IMAGE_VERSION;         output_png.format = PNG_FORMAT_GRAY;         output_png.width = image_width;         output_png.height = image_height;          /* write the png file */         png_image_write_to_file(&output_png, filename, 0, image_data, image_height, nullptr);          /* cleanup */         png_image_free(&output_png);     } } #endif 8  
Original en ingles

I have this code:

Tabzilla.fillZGContacts = function(){    if (!Tabzilla.panel.id) return;    $.ajax({        url: 'zgcontacts.json',       success: function(d){   // "Type","Name","Link","Contact","Location","Icon"         Tabzilla.zgContacts = d;         var countries = [];         d.rows.forEach(function(row){           if (row[0] == 'Country') countries.push(             {link:row[2], contact:row[3], country: row[4]}           );              });          //alphabetically         countries.sort(sortByKey('country'));          //adding link          countryTemplate = function (country){           s = '<a title="'+country.country+'" class="chapters_link" href="'           +country.link+'" target="_blank">'       +'<div class="chapters c_'+country.country.toLowerCase()+'">'         +'<span class="flag-margin">'+country.country+'</span></div></a>'          return s;         }           var byletter = {};          //count countries starting from each letter         countries.forEach(function(c){                   var firstletter = c.country.toLowerCase().charAt(0);           if (byletter[firstletter]) byletter[firstletter]++;           else byletter[firstletter]=1;         });         console.log(byletter);         //prepare containers         var panel = $("#"+Tabzilla.panel.id);         var $cols = [];           $cols.push(panel.find(".c_COL4"));         $cols.push(panel.find(".c_COL3"));         $cols.push(panel.find(".c_COL2"));         $cols.push(panel.find(".c_COL1"));         var columns = $cols.length;                 var targetlen = countries.length/columns;          var FirstLetter = countries[0].country.toLowerCase().charAt(0);         var cc = [];          //fill containers. this loop is buggy. should be reviewed.         countries.forEach(function(c){           var newFirstLetter = c.country.toLowerCase().charAt(0);           if (FirstLetter != newFirstLetter)           {               var l1 = cc.length;              var l2 = l1 + byletter[newFirstLetter];              //condition maybe shd be changed..               if (Math.abs(l2-targetlen) >= Math.abs(l1-targetlen)){                var $col;                if ($cols.length>0) $col = $cols.pop();                cc.forEach(function(c){                  $col.append(countryTemplate(c));                });                cc=[];                 //does not work :(                //could generate another template with first letter raised                $col.find('span').first().addClass("first-letter");              }              cc.push(c);           }           else cc.push(c);           FirstLetter = newFirstLetter;         });        },    }); } 

The zgcontacts.json file can be found here.

Any pointers in optimizing this is much appreciated. For example, this loop:

countries.forEach(function(c) 
     
         
         

Lista de respuestas

5
 
vote

Utilice los brizes { en todos sus bloques si. Hace que sea más fácil de leer.

  if (byletter[firstletter]){     byletter[firstletter]++; }else{     byletter[firstletter]=1; }   

TIENE for2 LOOPS LO GUSTE: countries.forEach(function(c){ que también son difíciles de leer. Solo usaría esto si tiene una función que desea aplicar.

  for(var countryindex = 0; countryindex < countries.length; countryindex++) {     var c = countries[countryindex];     // ... }   

Utilice nombres de variables descriptivos.

  if (FirstLetter != newFirstLetter) {     var l1 = cc.length;     var l2 = l1 + byletter[newFirstLetter];   

¿Qué es l1 & amp; l2 ? ¿Qué es c o cc ?

 

Use braces { in all your if blocks. It makes it easier to read.

if (byletter[firstletter]){     byletter[firstletter]++; }else{     byletter[firstletter]=1; } 

You have for loops like : countries.forEach(function(c){ which are also hard to read. I'd only use this if you have a function you want to apply.

for(var countryindex = 0; countryindex < countries.length; countryindex++) {     var c = countries[countryindex];     // ... } 

Use descriptive variable names.

if (FirstLetter != newFirstLetter) {     var l1 = cc.length;     var l2 = l1 + byletter[newFirstLetter]; 

What is l1 & l2 ? What is c or cc?

 
 
 
 
5
 
vote

La ubicación de su función rompe el flujo de su código, debe ponerlo al final. También le animaría a que use alguna rutina de plantación, podría usar este:

  if (byletter[firstletter]){     byletter[firstletter]++; }else{     byletter[firstletter]=1; } 1  

Luego, su país de campo podría ser:

  if (byletter[firstletter]){     byletter[firstletter]++; }else{     byletter[firstletter]=1; } 2  

Esto podría ser seco, por supuesto, ya que repito if (byletter[firstletter]){ byletter[firstletter]++; }else{ byletter[firstletter]=1; } 3 varias veces, dejo el código final para usted.

Además, no hay razón para no fusionar la extracción de los países y el cálculo de if (byletter[firstletter]){ byletter[firstletter]++; }else{ byletter[firstletter]=1; } 4 , le ahorrará una llamada 'foreach':

  if (byletter[firstletter]){     byletter[firstletter]++; }else{     byletter[firstletter]=1; } 5  

Finalmente, su LOOP BURCY está muy bien escrito, no puedo decir lo que está tratando de lograr con ese código. Tal vez usted debe comentar cada sección con lo que se supone que debe hacer y, de hecho, seguir las sugerencias de James Khoury.

 

The location of your countryTemplate function breaks the flow of your code, you should put it at the very end. Also I would encourage you to use some templating routine, you could use this one:

function fillTemplate( s ) { //Replace ~ in s with further provided arguments   for( var i = 1 ; i < arguments.length ; i++ )     s = s.replace( "~" , arguments[i] );       return s; }       

Then your countryTemplate could be :

 countryTemplate = function (country)  {    var template = '<a title="~" class="chapters_link" href="~" target="_blank">' +                     '<div class="chapters c_~">' +                       '<span class="flag-margin">~</span>' +                     '</div>' +                   '</a>';    return fillTemplate( template , country.country                                  , country.link                                  , country.country.toLowerCase()                                  , country.country   } 

This could be DRY'er of course, since I repeat country.country a number of times, I leave the final code to you.

Furthermore, there is no reason not to merge the extraction of the countries and the calculation of byletter, it will save you a 'forEach` call:

    var countries = [],         byletter  = {};      d.rows.forEach(function(row){       if (row[0] == 'Country'){         var country = { link:row[2], contact:row[3], country: row[4] };         var firstletter = row[4].toLowerCase().charAt(0);         countries.push( country );         byletter[firstletter] = ( byletter[firstletter] || 0 ) + 1;       }     }); 

Finally, your buggy loop is very badly written, I cannot tell what you are trying to achieve with that code. Maybe you should comment each section with what it is supposed to do and indeed follow the suggestions of James Khoury.

 
 

Relacionados problema

1  Sube y arrastra la imagen dentro de una imagen de máscara  ( Upload and drag the image inside a mask image ) 
Estoy permitiendo a los usuarios subir y arrastrar imágenes con este código. Dame una revisión de esto. Codepen $part = '@CRC := MD5(CONCAT_WS('#', COA...

3  Construyendo una tabla HTML utilizando JavaScript  ( Building an html table using javascript ) 
¿Una forma más legible para hacer esto? renderHtmlTable(function(tableItems) { var tableArray,_i,item,_len; tableArray = ['<table id = sampleTable ><...

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...

1  Actualización de una página web de DRUPAL con contenido basado en texto ingresado  ( Updating a drupal web page with content based on inputted text ) 
La intención es actualizar una página web de drupal existente con contenido basado en texto ingresado en un cuadro de texto. La página existente muestra dat...

2  Limpiando una galería de imágenes rotativas  ( Cleaning up a rotating image gallery ) 
He creado una pequeña galería de imágenes para la web. Me propuse querer 3 cosas. 1. Toda la galería tenía una capacidad de respuesta a nivel básica. 2. La ga...

3  Función de estilo sortable jquery  ( Jquery sortable style function ) 
Esta es una función de clasificación básica escrita en jQuery que mueve los elementos en la DOM alrededor para crear espacios vacíos para un gotpable (). Como...

3  Página web basada en una muestra de un libro  ( Web page based on a sample from a book ) 
He creado una página web basada en una muestra de un libro. Funciona bien, pero parece haber sido demasiado complicado. dt4 ¿Es posible mejorar la clari...

5  Función de movimiento para un juego  ( Move function for a game ) 
Tengo una función PackageSubpackageSubpackageName6 en este juego que estoy haciendo. Funciona bien, pero me gustaría reducir un poco su tamaño. PackageSu...

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...

8  Escribiendo un widget de jquery: plantilla  ( Writing a jquery widget templating ) 
Estoy haciendo mi primer mayor desarrollo de jQuery. Es un widget para eventos recurrentes, y es como una bestia bastante compleja. El código completo está di...




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