search

benrr101 / node-taglib-sharp

A node.js port of mono/taglib-sharp
GNU Lesser General Public License v2.1
42 stars 11 forks source link

Dependencies replace #98

Closed subframe7536 closed 11 months ago

subframe7536 commented 11 months ago

some dependencies maybe unnecessary

replace itiriri with Array.from

replace dateFormat with new DateUtils

benrr101 commented 11 months ago

As much as I can appreciate removing dependencies, there are some reasons why I opted to use itiriri. Array.from creates a copy of whatever was provided to it as input, while itiriri treats creates a chain of operations to perform on an iterable and only "materializes" the array when .toArray is called. This is very similar to LINQ behavior in C# and reduces the memory requirements when doing many operations or having several operations chained together. Do you think getting rid of itiriri and taking the memory hit outweighs leaving it in?

DateFormat on the other hand, I need to reassess where it's being used. Based on the the changes you suggested, I think we can remove some usages with .toIsoString and just drop microseconds and timezone.

subframe7536 commented 11 months ago

yes, after reading its doc and I think you are right.

I have modified my code, and now it is consistent with the original implementation correctly.

Perhaps removing those "less important" dependencies is just my personal quirk, feel free to close this PR, but I do think we can benifit from it.

benrr101 commented 11 months ago

Understood. I think this is worth taking. Though I might undo some of the dateformat changes.