search

jaime-olivares / zipstorer

A Pure C# Class to Store Files in Zip
MIT License
183 stars 63 forks source link

CA1028: Enum storage should be Int32 #40

Closed mind-bending-forks closed 3 years ago

mind-bending-forks commented 4 years ago

The FxCop static analysers are reporting CA1028 against the following line in ZipStorer.cs:

public enum Compression : ushort

These state that

Even though you can change this underlying type (from System.Int32), it is not necessary or recommended for most scenarios. No significant performance gain is achieved by using a data type that is smaller than Int32. If you cannot use the default data type, you should use one of the Common Language System (CLS)-compliant integral types, Byte, Int16, Int32, or Int64 to make sure that all values of the enumeration can be represented in CLS-compliant programming languages.

So, please consider making one of the following changes:

Thanks.

tw-JordanChen commented 3 years ago

If you cannot use the default data type, you should use one of the Common Language System (CLS)-compliant integral types, Byte, Int16, Int32, or Int64 to make sure that all values of the enumeration can be represented in CLS-compliant programming languages.

So, please consider making one of the following changes:

  • If the fact that it is a ushort specifically is relied upon and required, then change its underlying type to System.UInt16, otherwise
  • alter the code such that its underlying type is the default System.Int32.

According to the wiki, the compression method field in the header is 2 byte. If the author takes into account compatibility, this warning could be suppressed without any risk. Besides, ushort is just a syntactic sugar and it equals to UInt16, which is not a CLS-compliant integral type either.

jaime-olivares commented 3 years ago

A stated above, this field is not 32 bits but 16 bits. ushort or UInt is the right type to use for that field, according to the standard. You may disable that FXCop warning with a directive.