2
\$\begingroup\$

At our office there is this piece of code (the original author isn't employed anymore, so I can't ask him).

Does anybody have an idea, what he did there? Is there any advantage of using this struct OrderDirection instead directly the enum Direction?

(for me, this is interesting and I just wanted to know why.)

public struct OrderDirection 
{
    private enum Direction
    {
        Ascending = 0,
        Descending = 1
    }
    
    private OrderDirection(Direction direction)
    {
        _direction = direction;
    }
    
    private readonly Direction _direction;
    
    public override string ToString()
    {
        switch(_direction)
        {
            case Direction.Ascending: return "asc";
            case Direction.Descending: return "desc";
            default: throw new NotImplementedException(nameof(_direction));
        }
    }
    
    public static OrderDirection Ascending = new OrderDirection(Direction.Ascending);
    public static OrderDirection Descending = new OrderDirection(Direction.Descending);
}
\$\endgroup\$
6
  • 3
    \$\begingroup\$ First, it looks like a decently object-oriented (specifically, polymorphism) approach to letting the ToString() method do the real work of representing what the value is. You'd certainly need to find out where it's being used to get that answer, but that's my guess looking at it. Second, mark those public static values as readonly - don't need some random code changing what is assigned to Ascending or Descending. \$\endgroup\$ Jul 20, 2020 at 14:57
  • \$\begingroup\$ @JesseC.Slicer so (besides the missing readonly) this actually is good code? haha, sorry never seen someone before doing this. He uses it to build sql-queries with stringbuilder and things.. \$\endgroup\$ Jul 20, 2020 at 15:02
  • \$\begingroup\$ I see no need for the enum when a simple bool would suffice. The original programmer kept all other details hidden that all you should care about is using the static instances, and both of them can be easily be created without the enum. \$\endgroup\$
    – Rick Davin
    Jul 20, 2020 at 16:15
  • 2
    \$\begingroup\$ @MatthiasBurger It models the business domain very well, even if it is a little over the top for something this small. I'm not really a fan of the NotImplementedException. That shouldn't be necessary (nor the default label) because all enum values are covered in the switch labels. If you added a different enum value called "Unknown" with the zero value, I could see it. But as-is, no. \$\endgroup\$ Jul 20, 2020 at 16:38
  • 2
    \$\begingroup\$ @MatthiasBurger on the other hand, ToString() should never throw exceptions. \$\endgroup\$ Jul 20, 2020 at 16:45

2 Answers 2

3
\$\begingroup\$

In my opinion nothing indicate the use of enum.

According to my understanding your object provides the following functionalities from consumer perspective:

  • Provide two factory methods one for Ascending and another for Descending
  • Provide custom string representation (abbreviation) for these

From the public interface point of view the following implementation is equivalent with yours

public struct OrderDirection 
{
    private readonly string _direction;
    private OrderDirection(string direction)
    {
        _direction = direction;
    }
      
    public override string ToString() => _direction;
    
    public static OrderDirection Ascending = new OrderDirection("asc");
    public static OrderDirection Descending = new OrderDirection("desc");
}
\$\endgroup\$
1
\$\begingroup\$

as OOP as is, I think this would avoid unneeded castings, and it gives you more managed approach to handle enum than using enum directly.

If enum was used directly, it would be possible to cast it to different type than a string, and it will make your hands tight when it comes to handle exceptions. so using struct with an internal enum would force typing it to only accept and output specified types, which avoid unnecessary exceptions and give you more OOP options to work with.

This is also a security approach where it adds restrictions on the input-output. In this case, avoiding SQL Injections.

Obviously it can be done in different approaches, however, this implementation would avoid some of the human-errors (like misspelling), and overriding. It would also add more readability and maintainability to the code and avoiding redundancy with less code possible.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.