Breaking up a navigation menu

Question

This code works as I intended, but I am definitely a JS noob (C++ background), so I'm not sure if this is the best way to do it (for example using .each instead of some kind of a for loop was completely new to me).

https://github.com/TaylorHuston/menuBreak/blob/master/menuBreak.js

function menuBreak() {
var windowWidth = $(window).width();
var menuLength = 0;
$("#navUl > li").each( function (index, element) {
 menuLength += $(this).width();
});
if( menuLength > windowWidth*.9 ) {
 var newWidth= menuLength*.7;
 $('#navUl').css({"maxWidth":newWidth});
} else {
 $('#navUl').css({"maxWidth":windowWidth});
}
}

And in my HTML, not sure if this is the 'best' way to call it

<script>
 $(document).ready(function() {
 menuBreak();
 });
 $(window).resize(function() {
 menuBreak();
 });
</script>

Answer 1

From a once over:

• You could have used a loop instead of .each(), it would be marginally faster
• Your indenting is off, I assume you had trouble pasting the code
• .9 and .7 are magic constants, you should declare these with var and a good name

• I would use a ternary for this:

  if {
   var newWidth= ;
   $('#navUl').css({"maxWidth":newWidth});
  } else {
   $('#navUl').css({"maxWidth":});
  }
  

  could be

  var navWidth = ( menuLength > windowWidth *.9 ) ? menuLength *.7 : windowWidth;
  $('#navUl').css({"maxWidth":navWidth});
  

  of course with properly named constants this might stretch too much to be easily readable, up to you.

• With jQuery, the most common way to trigger on ready is

  $( function(){
   //Do Something
   menuBreak();
  });
  

  now, since $() expects a function, we might as well pass menuBreak immediately:

  $( menuBreak );
  

  The same things goes for resize:

  $(window).resize( menuBreak );