Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address issues raised by Marsel Yalalov #3

Open
abdulapopoola opened this issue Dec 9, 2015 · 0 comments
Open

Address issues raised by Marsel Yalalov #3

abdulapopoola opened this issue Dec 9, 2015 · 0 comments

Comments

@abdulapopoola
Copy link
Owner

  • isEmpty: hmm, it looks like I cannot create infinite stream of nulls ;-)
  • hasEmptyTail: obviously, to make it faster, you should check at first line if (this.isEmpty() || this.streamRest == null), or even this.streamFirst == null || this.streamRest == null. Then the only opportunity for tail() to fail is an exception in this.streamRest(), and I'm not sure if it should be handled. So, then you can remove try/catch block.
  • append (and others): returning recursive function is only method to handle infinite streams maybe, but what about possible memory leaks? Isn't there a way to avoid recursive?
  • pick: no check if n is not a number. Don't know, what is better to insert: n = n >>> 0 or some check.
  • elementAt: maybe it'd be better to replace index == null to !index, like you do it in pick()
  • maybe I'm wrong, but it seems like (new Stream(1, null)).reduce(function () {}) will fail because you do s.tail() twice.
  • reduce doesn't check incorrect value of fn, like in map().
  • map: maybe it'd be better to check typeof fn === 'function' instead of fn == null
  • filter: no check of fn
  • walk: why you check s.hasEmptyTail(), if the next check is s.isEmpty() ?
  • print: oh, debug methods should be responsibility of user, not library
  • remove: can be refactored as

function remove(n) {
var s = this;
while (n > 0 && s.isEmpty()) {
s = s.tail();
n--;
}
return new Stream(s.head(), return s.tail);
}

  • zip: incorrect work with tails. The length of values array should be constant, but now it reduces, when one of tails becomes empty. Empty tails should be inserted as empty tails, I think.
  • fromInterval, from, upTo: step wants to be here as third/second arguments :)
  • NaturalNumbers: consider please possibility to give the previous value (head(), actually) into tail() function. It would be very useful to avoid creating new Stream objects when generating simple streams.

General issues:

  • no checks for finite length of streams in methods, which requires finite length. Maybe to implement some cutoff?
  • chains of methods can be not effective, because requires several passes. If streams are long, it can be necessary from user to implement sum(), contains(), etc manually, to limit number of passes. Maybe it would be more convenient to add additional parameter, like .contains(val, num), which will be equal to .pick(num).contains(val), but in one pass.

https://www.linkedin.com/groups/121615/121615-6077302528133836800?trk=hb_ntf_LIKED_GROUP_DISCUSSION_YOU_CREATED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant