Mon code jQuery fonctionne, mais est-il très mauvais du sharepoint vue d’un programmeur?

J’ai bricolé cette fonction jQuery. Son but est de calculer les marges de tous les éléments img dans div.article afin d’équilibrer la hauteur de l’image avec la grid de base du document, laquelle est égale à 20 px. Afin de correspondre à ma grid de ligne de base, chaque hauteur d’image doit être un multiple de 20. Si ce n’est pas le cas, par exemple, la hauteur d’une image est de 154 px, la fonction ajoute une marge de 6 px à l’image img, de sorte que la balance avec la ligne de base la grid est restaurée.

La fonction fonctionne correctement , donc ma question est la suivante: étant donné que je ne suis pas un programmeur, je me demande si mon code est très mauvais, même s’il fonctionne, et si oui, comment pourrait-il être amélioré?

Le code jQuery:

 $('div.article img').each(function() { // define line height of CSS baseline grid: var line_height = 20; // capture the height of the img: var img_height = $(this).attr('height'); // divide img height by line height and round up to get the next integer: var img_multiply = Math.ceil(img_height / line_height); // calculate the img margin needed to balance the height with the baseline grid: var img_margin = (img_multiply * line_height) - img_height; // if calculated margin < 5 px: if (img_margin < 5) { // then add another 20 px to avoid too small whitespace: img_margin = img_margin + 20; } // if img has caption: if ($($(this)).next().length) { // then apply margin to caption instead of img: $($(this)).next().attr('style', "margin-bottom: " + img_margin + "px;"); } else { // apply margin to img: $(this).attr('style', "margin-bottom: " + img_margin + "px;"); } });

Exemple de code HTML, img avec légende:

     Image Caption Goes Here

Exemple de code HTML, img sans légende:

Edit: code raffiné basé sur les suggestions de Russ Cam:

 var line_height = 20; var min_margin = 5; $('div.article img').each(function() {  var $this = $(this); // prefixed variable name with $ to remind it's a jQuery object var img_height = $this.height(); var img_margin = ((Math.ceil(img_height / line_height)) * line_height) - img_height; img_margin = (img_margin < min_margin)? img_margin + line_height : img_margin; if ($this.next().length) { $this.next().css({'margin-bottom' : img_margin + 'px'}); } else { $this.css({'margin-bottom' : img_margin + 'px'}); } });

Quelques améliorations que je peux recommander

1.cache $(this) à l’intérieur de each() dans une variable locale

 $('div.article img').each(function() { var $this = $(this); // prefixed variable name with $ // to remind it's a jQuery object // .... if ($this.next().length) { // .... } });

2. au lieu de définir attr('style') , utilisez la commande css()

3.Pas besoin de le faire

 $($(this))

Bien que jQuery ne se casse pas, il est inutile de passer un object jQuery à un autre object jQuery.

4.Utilisez $(this).height() ou $(this).outerHeight() pour obtenir la hauteur de l’élément dans le navigateur.

5.Non spécifique à jQuery, mais peut utiliser l’opérateur conditionnel ternaire pour affecter cette valeur

 // if calculated margin < 5 px: if (img_margin < 5) { // then add another 20 px to avoid too small whitespace: img_margin = img_margin + 20; }

devient

 // if calculated margin < 5 px then add another 20 px // to avoid too small whitespace img_margin = (img_margin < 5)? img_margin + 20 : img_margin;

6. Comme Alex l'a noté dans les commentaires, je supprimerais également les commentaires superflus qui ne font que répéter ce que le code vous dit. Même s'il s'agit du script de débogage, à mon avis, ils réduisent la lisibilité et les commentaires ne doivent servir qu'à fournir des détails qui ne sont pas inhérents à la lecture du code.

Le code est bon. Vous pourriez apporter quelques améliorations mineures:

N’utilisez pas $(this) partout. Atsortingbuez-le à quelque chose de bonne heure et utilisez-le pour ne pas ré-étendre l’élément encore et encore.
$(this).attr('style', "margin-bottom: " + img_margin + "px;"); peut être réécrit sous la forme someEl.css('margin-bottom', img_margin + 'px');

La hauteur de l’image ne doit pas être extraite de l’élément, utilisez plutôt $ (this) .height, car c’est la hauteur réelle dans le navigateur.

Quoi qu’il en soit, il pourrait être réécrit beaucoup plus court, quelque chose comme

 $('div.article').each(function() { var img_margin = 20 - $(this).children('img:first').height() % 20; if(img_margin < 5) img_margin += 20; if($(this).children('small').length > 0) $(this).children('small').attr('style', "margin-bottom: " + img_margin + "px;"); else $(this).children('img').attr('style', "margin-bottom: " + img_margin + "px;"); }

Vous ne devriez pas commenter chaque ligne pour dire ce qui se passe, le code devrait vous dire ce qui se passe. (sauf si c’est une déclaration vraiment bizarre)
Les commentaires doivent être utilisés pour vous dire pourquoi quelque chose est fait.

par exemple:

 // if img has caption: if ($($(this)).next().length) { // then apply margin to caption instead of img: $($(this)).next().attr('style', "margin-bottom: " + img_margin + "px;"); } else { // apply margin to img: $(this).attr('style', "margin-bottom: " + img_margin + "px;"); }

pourrait être changé pour, ce qui est beaucoup plus lisible à mes yeux:

 // if img has caption, apply margin to caption instead if ($($(this)).next().length) { $(this).next().css('margin-bottom', img_margin + 'px;'); } else { $(this).css('margin-bottom', img_margin + 'px;'); }

Je pense que vous pouvez laisser tomber le

 $($(this))

en faveur de

 $(this)

Un moyen d’accélérer et de simplifier le calcul de la hauteur serait:

 var img_margin = 20 - ($this.height() % 20);

Cela devrait aider un peu au moins.

Voici ce que je ferais, explication dans les commentaires

 $(function(){ // put this variable out of the loop since it is never modified var line_height = 20; $('div.article img').each(function() { // cache $(this) so you don't have to re-access the DOM each time var $this = $(this); // capture the height of the img - use built-in height() var img_height = $this.height(); // divide img height by line height and round up to get the next integer: var img_multiply = Math.ceil(img_height / line_height); // calculate the img margin needed to balance the height with the baseline grid: var img_margin = (img_multiply * line_height) - img_height; // if calculated margin < 5 px: if (img_margin < 5) { // then add another 20 px to avoid too small whitespace: //use ternary operator for concision img_margin += 20; } // if img has caption: if ($this.next().length) { // then apply margin to caption instead of img: - use built-in css() function $this.next().css("margin-bottom", img_margin); } else { // apply margin to img: - use built-in css() function $this.css("margin-bottom", img_margin); } }); });