fbarresi / Sharp7

Nuget package for Sharp7
MIT License
208 stars 73 forks source link

Add method overloads with enums #11

Closed u1alive4ever closed 4 years ago

u1alive4ever commented 4 years ago

Sorry for my English.

In my work, I decided to use your code, but during the development of the project, a question arose. What is the reason for choosing to use constants instead of enum?

Example from wiki:

public int ReadArea(int Area, int DBNumber, int Start, int Amount, int WordLen, byte[] Buffer)

where:

public const byte S7AreaPE = 0x81; public const byte S7AreaPA = 0x82; public const byte S7AreaMK = 0x83; public const byte S7AreaDB = 0x84; public const byte S7AreaCT = 0x1C; public const byte S7AreaTM = 0x1D;

and

public const int S7WLBit = 0x01; public const int S7WLByte = 0x02; public const int S7WLChar = 0x03; public const int S7WLWord = 0x04; public const int S7WLInt = 0x05; public const int S7WLDWord = 0x06; public const int S7WLDInt = 0x07; public const int S7WLReal = 0x08; public const int S7WLCounter = 0x1C; public const int S7WLTimer = 0x1D;

The variables that we pass to the ReadArea method, such as Area and WordLen, are not obvious, and they are very easy to make mistakes in. You can convert the code as follows.

    public enum S7Area
    {
        PE = 0x81,
        PA = 0x82,
        MK = 0x83,
        DB = 0x84,
        CT = 0x1C,
        TM = 0x1D
    }

    public enum S7WordLength
    {
        Bit = 0x01,
        Byte = 0x02,
        Char = 0x03,
        Word = 0x04,
        Int = 0x05,
        DWord = 0x06,
        DInt = 0x07,
        Real = 0x08,
        Counter = 0x1C,
        Timer = 0x1D
    }

public int ReadArea(S7Area area, int DBNumber, int Start, int Amount, S7WordLength length, byte[] Buffer)

This way, you restrict the variables that you can pass while keeping their values. This is just a small example. there are a lot of constants in the code, starting from the block language and ending with "Error codes". What is the reason for this?

fbarresi commented 4 years ago

Hi,

This is a good first Issue, thank you for reporting it! I won't complete remove the overloads for the methods, but I would mark them as deprecated. I kept them in the code just for backward capability scopes and for an easy migration from snap7.

Best regards, Federico

fbarresi commented 4 years ago

Hi, I've fixed this issue in develop branch, I would like to add more internal usages of those two enums internally than I'll merge this feature in master. Best regards, Federico