0
\$\begingroup\$

To avoid the use of magic numbers, I created a static class to store some integer values that are represented inside a database. I avoided using enums because I need to cast them every time I use them. To benefit from the clean approach of enums while also enjoying the type definition offered by classes.

The Class Approach

public static class TransactionType
{
    public static readonly int
        Deposit = 1,
        Withdrawal = 2,
        Transfer = 3,
        Payment = 4,
        Refund = 5;
}

// Example Usage
public void ProcessTransactionClass(int transactionTypeId)
{
    if (transactionTypeId == TransactionType.Deposit)
    {
        // Handle deposit
    }
    else if (transactionTypeId == TransactionType.Withdrawal)
    {
        // Handle withdrawal
    }
    // ... and so on for other types
}

Enum Approach

public enum TransactionType
{
    Unassigned = 0, // Default value
    Deposit = 1,
    Withdrawal = 2,
    Transfer = 3,
    Payment = 4,
    Refund = 5
}

// Example Usage
public void ProcessTransactionEnum(int transactionType)
{
    if (transactionType == (int)TransactionType.Deposit)
    {
        // Handle deposit
    }
    else if (transactionType == (int)TransactionType.Withdrawal)
    {
        // Handle withdrawal
    }
    // ... and so on for other types
}

I haven't seen this kind of usage anywhere, is this style any good and acceptable?

\$\endgroup\$
7
  • 1
    \$\begingroup\$ This lacks context. Can you also show example code where you use a TransactionType? Also, any reason why you did not make them const? I wonder if your resistance on casting to an enum to an int is more a lack of understanding on your part. Example usage code will be a big help in getting quality answers. \$\endgroup\$ Commented Dec 5, 2023 at 16:55
  • \$\begingroup\$ enums because I need to cast them every time => perhaps a Dictionary w/ enum members as the key. \$\endgroup\$ Commented Dec 5, 2023 at 17:01
  • \$\begingroup\$ @RickDavin I provided an example. Using const the value will be stored inside the assembly which I don't want for various reasons. Like a change in a shared library would result in different values because it is determined at compile time \$\endgroup\$ Commented Dec 5, 2023 at 17:12
  • \$\begingroup\$ @radarbob Sorry, I really can't see the benefit here, could you elaborate please? \$\endgroup\$ Commented Dec 5, 2023 at 17:23
  • \$\begingroup\$ @Abcd, can you elaborate on why you won't cast enums. I assume you want to avoid the computation overhead. \$\endgroup\$ Commented Dec 5, 2023 at 17:36

3 Answers 3

Reset to default
1
\$\begingroup\$

Ah, after much back and forth I think I understand....

I haven't seen this kind of usage anywhere, is this style any good and acceptable?

No. No it's not. Be consistent with Type usage. If these numbers are "kinds of transactions" then write the code that way. Do not flip flop all over the code base, it induces bug risk.

Use enums. They are Type safe. Mistakes are caught at compile time. Cast the DB sourced values to enums, not the other way around. Do this up front, probably in a constructor, THEN use them.

\$\endgroup\$
3
  • \$\begingroup\$ Aren't class fields type safe too? \$\endgroup\$ Commented Dec 5, 2023 at 18:37
  • 1
    \$\begingroup\$ @Abcd they're base type safe (int in this case), but they're not context-safe. Nothing stops you from writing addressTypeId = TransactionType.Deposit - they're both ints, but their contexts are completely different and assigning ints everywhere isn't safe. Use the enum and convert at the last mile. \$\endgroup\$ Commented Dec 5, 2023 at 22:04
  • \$\begingroup\$ shameless plug for an ID (integer, long, guid) that has an affinity to a type: softwareengineering.stackexchange.com/a/159999/987 \$\endgroup\$ Commented Dec 5, 2023 at 22:45
3
\$\begingroup\$

I know that @radarbob says that your "class" in neither good style or acceptable, but I disagree. NOTE: this is NOT a critique of radarbob's answer. Rather, I am offering alternative perspective.

I have used some 3rd party C# SDK's that integrate with SQL Server tables. They do like your originally had by using named constants rather than enums, as this more tightly integrates with the integers coming over from SQL. I have seen it before, and do consider it acceptable.

The one thing I would change is to use const over static readonly.

\$\endgroup\$
3
  • \$\begingroup\$ I wonder the reasoning behind using const instead of readonly field. In another comment I expressed my concern and would like to know better. \$\endgroup\$ Commented Dec 5, 2023 at 23:25
  • 1
    \$\begingroup\$ @Abcd for your use case, it's better to use const instead. As the values are fixed, and they won't be changed. Unlike static readonly that used with values that needed to be assigned at runtime such as fetching the a values from the configurations. \$\endgroup\$ Commented Dec 6, 2023 at 3:26
  • \$\begingroup\$ more tightly integrates with the integers coming over => This is all well and good, it sounds like a fancy ORM. An ORM being a code layer whose Single Responsibility is to transliterate Domain values to/from DB form, effectively decouples the domain model from the storage medium. As long as you are not corrupting the model's Domain Specific Language with terms, values, types, and code workarounds for the sake of the storage medium \$\endgroup\$ Commented Dec 27, 2023 at 22:00
2
\$\begingroup\$

I haven't seen this kind of usage anywhere, is this style any good and acceptable?

enum Approach

If you mean the if else if structure... a switch statement will be more concise, easier to read, and quicker to understand.

THIS

public void ProcessTransactionEnum(int transactionType)

SHOULD BE

public void ProcessTransactionEnum(TransactionType transactionType)
  if (transactionType == TransactionType.Deposit)  { ...

And enums can be compared without casting to the underlying int values.

Here is an example touching on all the points below.

You don't need to specify the static on enum. enum is static by default. See this SO thread

Assigning explicit values to the enum members can be very beneficial. I like to make the enum default (zero) to be "unassigned", "unknown", etc. It is great for error checking and it avoids making initially wrong assignments, which is potential trouble.

I avoided using enums because I need to cast them every time I use them.

Casting is process-expensive. So what? I don't see the need in the context of "transaction types." In any case unless saving mere milliseconds is execution critical, I say take advantage of the coding happiness it gives to you, the reader, and maintenance programmer.

\$\endgroup\$
4
  • \$\begingroup\$ I think there is a misunderstanding here. The example I provided is the class approach I wanted to replace with enums. I will edit my post to provide both versions and be clearer. \$\endgroup\$ Commented Dec 5, 2023 at 17:31
  • \$\begingroup\$ It is not just about the time saving, it also clutters the code with unnecessary syntax. \$\endgroup\$ Commented Dec 5, 2023 at 17:36
  • \$\begingroup\$ Currently working with a database model that is handled by edmx, which doesn't support enum types, we have to cast either the Lvalue or the Rvalue to use them as function parameters etc. Your proposal seems good as a general solution but not so good in this case. \$\endgroup\$ Commented Dec 5, 2023 at 17:58
  • 1
    \$\begingroup\$ [the DB ... doesn't support enum types]. => That is irrelevant. The C# code is your "model", "domain". Write the code in that context. The DB is merely a storage medium and should not be influencing business model design. Coupling by omission, I'd call it. \$\endgroup\$ Commented Dec 5, 2023 at 18:12

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.