Skip to content
This repository was archived by the owner on Mar 14, 2022. It is now read-only.

Conversation

@jseijas
Copy link
Contributor

@jseijas jseijas commented Aug 27, 2018

Refactor of the code

Objectives

  • Update dependencies to not have audit alerts.
  • Increase maintainability by reducing duplicate code
  • Increase readability by reducing complexity and reducing area of interest

API Changes

There are no breaking changes, so there is retrocompatibility

…ability.

- Created iterate and iterate-promise functions as helpers for the others. The iterate is a foreach that also receives a function of "what to do when a value is getted" and "what to do when returning result". That way a filter creates a result array, when the value (boolean) is true then insert current element to result, and at the end return the array. The same for map, but when got a value (any) push it directly to result. Increases the maintainability due to reduce 3 different methods (foreach, map and filter) to a single point of maintenance (iterate).
- Simplified the promises to use the callback version, so increases maintainability because is one single point of modification for promise and callback version.
- Changed foreach promise test to check result values.
- Added foreach test to check result values after timeout to check the "no callback provided" case.
- Simplified the logic of the iteration to don't have a previous function call, so the logic is executed always inside next function.
});
});

it('should iterate all the elemnts even if no callback is provided', function(done) {

Choose a reason for hiding this comment

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

'it' is not defined.

it('should iterate all the elemnts even if no callback is provided', function(done) {
const result = [];
arrForeach(sampleArr,(el, i, arr, next) => { result.push(el); next(); });
setTimeout(() => {

Choose a reason for hiding this comment

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

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
'setTimeout' is not defined.


it('should iterate all the elemnts even if no callback is provided', function(done) {
const result = [];
arrForeach(sampleArr,(el, i, arr, next) => { result.push(el); next(); });

Choose a reason for hiding this comment

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

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

});

it('should iterate all the elemnts even if no callback is provided', function(done) {
const result = [];

Choose a reason for hiding this comment

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

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).


it('should resolve without error', function(done) {

const result = [];

Choose a reason for hiding this comment

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

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

@@ -1,46 +1,24 @@
const iterate = require('./iterate');

Choose a reason for hiding this comment

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

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

result.push(value);
}
},
() => result

Choose a reason for hiding this comment

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

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

return iterate(
arr,
func,
(value) => {

Choose a reason for hiding this comment

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

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

});
}
});
const result = [];

Choose a reason for hiding this comment

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

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

@@ -1,45 +1,23 @@
const iterate = require('./iterate-promise');

Choose a reason for hiding this comment

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

'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

@coveralls
Copy link

coveralls commented Aug 27, 2018

Coverage Status

Coverage increased (+0.8%) to 99.219% when pulling 8e45df3 on jseijas:feature/refactor into d584d77 on ashraful-islam:master.

}
return onComplete();
}
func(arr[i], i, arr, (err, data) => {

Choose a reason for hiding this comment

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

'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

@jseijas
Copy link
Contributor Author

jseijas commented Aug 27, 2018

Hi! Sorry about HoundCI issues, I never used that before neither XO (I'm more on the eslint + airbnb + linting in travisCI). Perhaps you can guide me how to get HoundCI happy.

@ashraful-islam
Copy link
Owner

Hi! Thanks for the contribution. It's very old module, was a quick put-together for an internal project which later got ported to Java. So, don't intend to update, will let it be.

@ashraful-islam ashraful-islam merged commit 5c47e10 into ashraful-islam:master Aug 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants