:sparkles: Question related to Commands and Segment class

[crapper]

Is your feature request related to a problem? Please describe. After reading the structure of the AddSegment, ConvertSegment, and Segment classes, I have the following concerns about these three classes:

1. For AddSegment, I believe that using a variable variant to determine what will be executed violated SRP, I believe there should be two command classes called AddLinearSegment and AddCurveSegment instead of the current one.

2. For ConvertSegment, I believe that the function convertToLine should be renamed as convertToLinear

3. Furthermore, there is a section for checking the existence of the target modify segment in the path for function convertToCurve, https://github.com/Jerrylum/path.jerryio/blob/42f066990269a61df698f3aa008047150621c86d/src/core/Command.ts#L500-L503 But this section does not exist in covertToLine https://github.com/Jerrylum/path.jerryio/blob/42f066990269a61df698f3aa008047150621c86d/src/core/Command.ts#L496-L498 Please investigate whether this validation is required or not for both of function

4. For the Segment class, I believe there should contain functions that are similar to the following suggestion

   IsCurve(): boolean{
   return this.controls.length === 4
   }
   IsLinear(): boolean{
   return this.controls.length === 2
   }

   And modify the usage of checking the type of line for the segment, for example, https://github.com/Jerrylum/path.jerryio/blob/42f066990269a61df698f3aa008047150621c86d/src/app/SegmentPointsHitBoxElement.tsx#L47 to if (props.segment.isLinear)

5. For the enum of SegmentVariant, I suggest implementing it as the following:

   export enum SegmentVariant {
   Linear =2
   Cubic = 4
   }

   To prevent any direct comparison with the string that may appear in the future, which may harm the readability of the codes. (I know currently the actual value of the enum does not bother for the code readability).

[Jerrylum]

For 1, I agree that we should have two commands, AddLinearSegment and AddCurveSegment.

[Jerrylum]

For 2, I also agree

[Jerrylum]

For 3, I think this is because this.path.segments[index...] is needed in convertToCubic, but not in convertToLine. I think we can just keep it

[Jerrylum]

For 4, I will add them to the Segment class.

[Jerrylum]

For 5, I think I can change some of though

[Jerrylum]

Resolved