search

IcySnex / Melora

Melora allows you to download all your music from any platform using custom plugins.
https://icysnex.github.io/Melora/
Other
9 stars 1 forks source link

Fix: Folders via Filename format #1

Closed rasmus-z closed 2 months ago

rasmus-z commented 2 months ago

ToLegitFileName replaced '\' with '_'. That made folders via Filename format not work.

IcySnex commented 2 months ago

First of all, thank you for pointing out this problem. I must have overlooked it when testing.

I have run some benchmarks between different variations on how that issue could be fixed, including your solution: benchmark

This said, LINQ doesnt perform great - it's over 4x slower and allocates about 30x more memory than a simple if-check:

foreach (char c in Path.GetInvalidFileNameChars())
    if (c != '\\')
        fileName = fileName.Replace(c, '_');

return fileName;

An even better solution would be to simply store an own array including all invalid filename chars except '\', which I ended up doing.

Of course this is micro-optimizing and has little to no benefit in real usage but since the function is called on every single track being downloaded I think it wouldn't hurt to make it just a little faster.

IcySnex commented 2 months ago

I have made a pull request on the fork with the improvements. Please accept it so I can merge it with the main repo and push an update to Melora.

rasmus-z commented 2 months ago

Ah yes, sure might be a bit micro-optimization-y but since the optimization isn't complicated to implement nor use I definitely see it's worth. I've merged the pull request on the fork.