hapijs / hoek

Node utilities shared among the extended hapi universe
Other
480 stars 171 forks source link

add #Hoke.forEach? #112

Closed mateusvahl closed 9 years ago

mateusvahl commented 9 years ago

Hi, several times found loops like:

for (var i = 0, il = err.details.length; i < il; ++i) {
//...
}

we could make more intuitive/readable using functions, just looking the ".forEach"already know exactly what type of loop is:

var forEach = Hoek.forEach;

forEach(err.details, function(i) {
//...
});

forEach(err.details, (i) => {
//...
});

make any sense?

gergoerdosi commented 9 years ago

I don't see the reason to add it. How would it be different to Array.prototype.forEach()?

err.details.forEach(function (detail) {

    console.log(detail.path);
});
hueniverse commented 9 years ago

for...each loops are significantly slower. We do not allow them in any hot code (basically ok only in config and setup functions).

mateusvahl commented 9 years ago

@gergoerdosi for vs .forEach https://gist.github.com/mateuspv/23491da937cb4afec967 @hueniverse Maybe I have expressed myself wrong, we continue with for beneath the scenes, with less boilerplate(I think).

Hoek.forEach = function (arr, fn) {
    for (var i = 0, arrL = arr.length; i < arrL; ++i) {
        fn(i);
    }
};

performance apparently did not suffer change, perhaps needed a better test. https://gist.github.com/mateuspv/a7d100bd87d3f4fe705b

anyway, might just be a different view.

gergoerdosi commented 9 years ago

@mateuspv I see. I thought you only need the semantics. If performance is the same for for and Hoek.forEach() I think it would make sense to add it. I would modify it a little bit though to make it similar to Array.prototype.forEach() and pass the element too (most of the times we need the element, not the index).

Hoek.forEach = function (arr, callback) {

    for (var i = 0, il = arr.length; i < il; ++i) {
        callback(arr[i], i);
    }
};
hueniverse commented 9 years ago

We are not adding utilities to hoek that are not used internally by hapi. This is not a general purpose module.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.