search

panzerdp / voca

The ultimate JavaScript string library
https://vocajs.pages.dev
MIT License
3.61k stars 136 forks source link

Changes to Repeat and Reverse #17

Closed kiraknowsbest closed 7 years ago

kiraknowsbest commented 7 years ago

Description

Repeat: Removed while loop, used repeat method of String.prototype to improve readability Reverse: Changed function to increase time complexity from O(3n) to O(n)

Check List

panzerdp commented 7 years ago

Hi @kiraknowsbest!

Thanks for the pull request.

However:

Thanks for your time.

kiraknowsbest commented 7 years ago

Hi,

I respect you decision fully. I understand the reasoning for not using repeat(), and your preference for functional programming is completely understandable as well. Very interesting library, nice work!

On Wed, Dec 21, 2016 at 3:34 AM, Dmitri Pavlutin notifications@github.com wrote:

Hi @kiraknowsbest https://github.com/kiraknowsbest!

Thanks for the pull request.

However:

  • repeat() method is a part of ES2015 specification. Because the library targets ES5, it would require to include a polyfill for it: and I don't want that. The current while () {...} implementation does not create this dependency.
  • reverse(): in some situations the proposed code may perform faster. But I'm ok with the current implementation, since I like the functional programming approaches. Constants in time complexity are skipped θ(3n) = θ(n) anyway, and I am not interested in micro-optimizations.

Thanks for your time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/panzerdp/voca/pull/17#issuecomment-268501465, or mute the thread https://github.com/notifications/unsubscribe-auth/AQC90cr5Y_Lj_kpS09nwPHIYc2CGymQQks5rKQ7KgaJpZM4LS0fG .

pwolaq commented 7 years ago

How about using better version of repeat like the one here: http://www.2ality.com/2014/01/efficient-string-repeat.html

panzerdp commented 7 years ago

@pwolaq Actually this algorithm is implemented in Voca's repeat() :repeat:

kiraknowsbest commented 7 years ago

Yeah, it seems this algorithm is a bit more witty than it appears!

On Wed, Dec 21, 2016 at 3:51 AM, Dmitri Pavlutin notifications@github.com wrote:

@pwolaq https://github.com/pwolaq Actually this algorithm is implemented in Voca's repeat() 🔁

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/panzerdp/voca/pull/17#issuecomment-268504439, or mute the thread https://github.com/notifications/unsubscribe-auth/AQC90ThwLjGe8dRUEPamF3uv9gfD9rUzks5rKRKpgaJpZM4LS0fG .

panzerdp commented 7 years ago

Thanks @kiraknowsbest! If you have suggestions, feel free to open issues for discussion.