Skip to main content
Code Review

Return to Answer

replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

Ok, first off: Passing in console.log as a paramater seems unnecessary. The function should have one task:

Generate the skips

Then once you have the skips you can do whatever you want with it. So:

function skip(str, cb) -> function skip(str)

and:

skip('hello!', console.log); -> console.log(skip('hello'));

I would also not use a function within a function. I would try keeping the functions relatively small. Split the functions Split the functions:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res;
}
function skip(str) {
 var input = str.split('')
 return input.map(skips);
}

Also, given that str.split('') isn't too much to explicitly write out, change:

 var input = str.split('')
 return input.map(skips);

To just:

 return str.split('').map(skips);

Also, your code doesn't run exactly as described. Add a .join('') to return res; and this should fix it:

return res; -> return res.join('');

All in all:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res.join('');
}
function skip(str) {
 return str.split('').map(skipHelper);
}
console.log(skip('hello!'));

There are probably some Javascript language specific stylistic choices I am missing, but this is for general code structure.

Ok, first off: Passing in console.log as a paramater seems unnecessary. The function should have one task:

Generate the skips

Then once you have the skips you can do whatever you want with it. So:

function skip(str, cb) -> function skip(str)

and:

skip('hello!', console.log); -> console.log(skip('hello'));

I would also not use a function within a function. I would try keeping the functions relatively small. Split the functions:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res;
}
function skip(str) {
 var input = str.split('')
 return input.map(skips);
}

Also, given that str.split('') isn't too much to explicitly write out, change:

 var input = str.split('')
 return input.map(skips);

To just:

 return str.split('').map(skips);

Also, your code doesn't run exactly as described. Add a .join('') to return res; and this should fix it:

return res; -> return res.join('');

All in all:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res.join('');
}
function skip(str) {
 return str.split('').map(skipHelper);
}
console.log(skip('hello!'));

There are probably some Javascript language specific stylistic choices I am missing, but this is for general code structure.

Ok, first off: Passing in console.log as a paramater seems unnecessary. The function should have one task:

Generate the skips

Then once you have the skips you can do whatever you want with it. So:

function skip(str, cb) -> function skip(str)

and:

skip('hello!', console.log); -> console.log(skip('hello'));

I would also not use a function within a function. I would try keeping the functions relatively small. Split the functions:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res;
}
function skip(str) {
 var input = str.split('')
 return input.map(skips);
}

Also, given that str.split('') isn't too much to explicitly write out, change:

 var input = str.split('')
 return input.map(skips);

To just:

 return str.split('').map(skips);

Also, your code doesn't run exactly as described. Add a .join('') to return res; and this should fix it:

return res; -> return res.join('');

All in all:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res.join('');
}
function skip(str) {
 return str.split('').map(skipHelper);
}
console.log(skip('hello!'));

There are probably some Javascript language specific stylistic choices I am missing, but this is for general code structure.

Source Link
Dair
  • 6.2k
  • 1
  • 21
  • 45

Ok, first off: Passing in console.log as a paramater seems unnecessary. The function should have one task:

Generate the skips

Then once you have the skips you can do whatever you want with it. So:

function skip(str, cb) -> function skip(str)

and:

skip('hello!', console.log); -> console.log(skip('hello'));

I would also not use a function within a function. I would try keeping the functions relatively small. Split the functions:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res;
}
function skip(str) {
 var input = str.split('')
 return input.map(skips);
}

Also, given that str.split('') isn't too much to explicitly write out, change:

 var input = str.split('')
 return input.map(skips);

To just:

 return str.split('').map(skips);

Also, your code doesn't run exactly as described. Add a .join('') to return res; and this should fix it:

return res; -> return res.join('');

All in all:

function skipHelper(val, index, arr) {
 var res = [];
 var i = index;
 var iterator = i + 1;
 for (i; i < arr.length; i += iterator){
 res.push(arr[i]);
 }
 return res.join('');
}
function skip(str) {
 return str.split('').map(skipHelper);
}
console.log(skip('hello!'));

There are probably some Javascript language specific stylistic choices I am missing, but this is for general code structure.

default

AltStyle によって変換されたページ (->オリジナル) /