Discussion: Regarding Streamlining Structs and Enums in the code base

[agentjill]

Hey @MathNya, this issue is centered around some of the Structs and Enums

• In Enum CellRawValue, what is the difference between Str Variant and String Variant. I dont find any difference in code, so why dont just delete the redundunt variant.
• In Enum CellRawValue, Kindly also explain the need for Inline Variant when Inline strings are stored as strings anyway.
• In Enum CellRawValue, the Error Variant in my opinion needs to be more explicit, Now it only supports #VALUE! error, but there can also be different errors such as #REF!,#NUM!, #NULL!, #NAME?, #N/A and #DIV/0!. I know these are referenced in src/helper/formula.rs but this is in my opinion not properly ironed out. Therefore another struct that explictly represents error can be envisaged (say like CellErrorType).
• The error Enum XlsxError is placed seperately placed in src/reader/xlsx.rs, src/writer/csv.rs and src/writer/xlsx.rs with identical code, therefore a single error can be used which is referenced by all the concerned locations.

I hope with this issues sorted out the codebase would be more robust and capable than ever. I will be glad to work on the changes if you provide your opinion on them.

[MathNya]

@agentjill Thank you for contacting us. We will respond in order.

In Enum CellRawValue, what is the difference between Str Variant and String Variant. I dont find any difference in code, so why dont just delete the redundunt variant. Str was not needed. Let's remove it.

The Open XML specification had SharedString("s") and String("str"), so I prepared two, but umya-spreadsheet uses String in both cases, so Str is not needed. https://learn.microsoft.com/en-us/dotnet/api/documentformat.openxml.spreadsheet.cellvalues?view=openxml-3.0.1

[MathNya]

In Enum CellRawValue, Kindly also explain the need for Inline Variant when Inline strings are stored as strings anyway.

Yes. For the same reason, this is also unnecessary.

[MathNya]

In Enum CellRawValue, the Error Variant in my opinion needs to be more explicit, Now it only supports #VALUE! error, but there can also be different errors such as #REF!,#NUM!, #NULL!, #NAME?, #N/A and #DIV/0!. I know these are referenced in src/helper/formula.rs but this is in my opinion not properly ironed out. Therefore another struct that explictly represents error can be envisaged (say like CellErrorType).

You are correct. Umya-spreadsheet is still in its infancy regarding the implementation of errors. It may need to be modified according to your suggestion.

[MathNya]

The error Enum XlsxError is placed seperately placed in src/reader/xlsx.rs, src/writer/csv.rs and src/writer/xlsx.rs with identical code, therefore a single error can be used which is referenced by all the concerned locations.

This is also as you suggest. The initial implementation was still in place.

[schungx]

In Enum CellRawValue, Kindly also explain the need for Inline Variant when Inline strings are stored as strings anyway.

Yes. For the same reason, this is also unnecessary.

Would it be better to keep the variants for round-trip purposes?

Otherwise the output file might be much larger than the original if all shared strings are turned to individual copies.

[MathNya]

@schungx Sorry for being so confusing. All strings are converted to shared strings when they are written. The size of the file should therefore be smaller than the original file.

[agentjill]

In Enum CellRawValue, Kindly also explain the need for Inline Variant when Inline strings are stored as strings anyway.

Yes. For the same reason, this is also unnecessary.

Would it be better to keep the variants for round-trip purposes?

Otherwise the output file might be much larger than the original if all shared strings are turned to individual copies.

@schungx, In my original question I meant the variant Inline is not at all used in the library. However, the inline strings which it was originally meant to be used are stored as String variant of CellRawValue. This doesnot imply that the library dont consider inline strings, there are other structs in the library for doing that precisely.

I hope the explaination makes sense. Thanks for engagement.