Code Review

[Falke-Design]

Remove: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/convert/pathToRelative.js#L62-L67

Move Line 79 to 94: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/convert/pathToRelative.js#L79-L97

Remove: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/parser/parsePathString.js#L30-L35

Remove: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/util/getPropertiesAtPoint.js#L5

Remove: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/util/getPropertiesAtPoint.js#L18

Remove: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/util/getSegmentAtLength.js#L10-L12

Remove: https://github.com/thednp/svg-path-commander/blob/fd721051fb0718b646d7bc6338c2842167a9aaed/src/util/getSegmentOfPoint.js#L11-L13

I will do some tests later

[thednp]

Thank you, I need to know the FUNCTIONALITY is 100% covered by tests, and 100% valid, does it break your application or not, generally stuff like that.

The code cleanup is coming no problem.

Thanks again.

[thednp]

@Falke-Design about the

Move Line 79 to 94: svg-path-commander/src/convert/pathToRelative.js

That won't be possible

..\svg-path-commander\src\convert\pathToRelative.js
78:9 error Unexpected lexical declaration in case block no-case-declarations

[Falke-Design]

@thednp Looks fine for me but I use only a few functions in my program.

I went through the changes and looked into the test. Very nice refactoring. It's mind blowing how you have managed to get the same results as before with removing so much lines and adding so less lines 😄

I personally don't like in the cypress test importing the code from the src, because it would be more realistic if it runs with the generated source file from dist. Additionally it is easier to replace it with a source file of a different version by simply changing the url. Here is how I add the source code for the cypress test html. But it is no problem if you don't change that.

[thednp]

I wanted to have it create the coverage map of all files, I also prefer a single file import, but anyways. I found during the e2e and unit testing that many many lines/branches never get executed and cannot be tested, so I removed them all redundant branches/lines. But it's not only same results, it's much better and consistent results (I will make some demos to show how getPointAtLength or getTotalLenth is one of the most accurate in all scripts available now).

I'm very happy with the segmentArcFactory rework :) !!

The only thing left for us to do is to find how to do 3d transformation for Arc segments. Right now we're converting Arc segments to CubicBezier for transformations. We might also feature something like tangent at point/length idk.

Let me know if you're interested.

[Falke-Design]

The only thing left for us to do is to find how to do 3d transformation for Arc segments. Right now we're converting Arc segments to CubicBezier for transformations. We might also feature something like tangent at point/length idk. Let me know if you're interested.

Sounds very nice, the problem is that I can't help you. This much over my knowledge and understanding of path transformations. 🙁

[thednp]

Alright, we can close this now. Thanks for your feedback.