Skip to content

Commit

Permalink
Breaking loops style
Browse files Browse the repository at this point in the history
  • Loading branch information
@megawac
megawac committed Jun 22, 2014
1 parent 1b63049 commit 74f58c1
Showing 1 changed file with 6 additions and 31 deletions.
37 changes: 6 additions & 31 deletions lodash.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,11 +384,7 @@
var index = -1,
length = string.length;

while (++index < length) {
if (chars.indexOf(string.charAt(index)) < 0) {
break;
}
}
while (++index < length && chars.indexOf(string.charAt(index)) >= 0) {}
return index;
}

Expand All @@ -403,11 +399,7 @@
*/
function charsRightIndex(string, chars) {
var index = string.length;
while (index--) {
if (chars.indexOf(string.charAt(index)) < 0) {
break;
}
}
while (index-- && chars.indexOf(string.charAt(index)) >= 0) {}
return index;
}

Expand Down Expand Up @@ -1062,12 +1054,7 @@
function arrayEach(array, iterator) {
var index = -1,
length = array ? array.length : 0;

while (++index < length) {
if (iterator(array[index], index, array) === false) {
break;
}
}
while (++index < length && iterator(array[index], index, array) !== false) {}
return array;
}

Expand All @@ -1082,11 +1069,7 @@
*/
function arrayEachRight(array, iterator) {
var length = array ? array.length : 0;
while (length--) {
if (iterator(array[length], length, array) === false) {
break;
}
}
while (length-- && iterator(array[length], length, array) !== false) {}
return array;
}

Expand Down Expand Up @@ -1524,11 +1507,7 @@
if (support.unindexedChars && isString(iterable)) {
iterable = iterable.split('');
}
while (++index < length) {
if (iterator(iterable[index], index, collection) === false) {
break;
}
}
while (++index < length && iterator(iterable[index], index, collection) !== false) {}
} else {
baseForOwn(collection, iterator);
}
Expand All @@ -1552,11 +1531,7 @@
if (support.unindexedChars && isString(iterable)) {
iterable = iterable.split('');
}
while (length--) {
if (iterator(iterable[length], length, collection) === false) {
break;
}
}
while (length-- && iterator(iterable[length], length, collection) !== false) {}
} else {
baseForOwnRight(collection, iterator);
}
Expand Down

15 comments on commit 74f58c1

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdalton do you dig this style change or are you partial to break? Theres plenty of other places this can be changed as well

@jdalton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the break.
For me it's more readable, when minifiers get a hold of it they tend to transform it into something like you've done.

@jdalton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ya wanna help get lodash bumped I'd love some help porting my php jsdoc->markdown converter to JavaScript.
See https://github.com/jdalton/docdown/tree/js.

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, personally I hate it :). Might take a look at it but I'm wrapped up in a bunch of projects already

One note is I doubt minifiers will be able to coerce the break implementation for the forIn and difference family

@jdalton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, personally I hate it :)

I'll mull it over, I do that expression combining in methods like takeWhile and friends.

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of takeWhile, you have an unnecessary variable in that function. You can rewrite it:

function takeWhile(array, predicate, thisArg) {
      var index = -1,
          length = array ? array.length : 0;

      predicate = lodash.callback(predicate, thisArg, 3);
      while (++index < length && predicate(array[index], index, array)) {}
      return slice(array, 0, index < 0 ? 0 : index);
}

@jdalton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch.

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want a pr or you want to handle it?

@jdalton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naw, I got it already in local. That's the advantage of not just being a repo sitter, things get done faster.

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On another note @jdalton, would you be open to a zipMin implementation. Could easily accomplish it in an extra 4 or so loc via a wrapper

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in the opposite school for the the jashkenas/underscore#1237 discussion

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll write a quick implementation without comments and leave it up to you

@jdalton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked the way #1237 ended, not destroying data is a win.

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's something to both arguments which is why I think they should both be enablable (or whatever). I think this comment said it best jashkenas/underscore#1237 (comment)

@megawac
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick implementation 2b0dd82

Please sign in to comment.