HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

Validation Error Handling - DRY Pattern Implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingerrorvalidationdryimplementationpattern

Problem

The Country, State and City functions accepts the Integer as Parameter and returns the relevant error code if the validation fails in ValidateNotExist Class.

Therefore, I have enhanced the ValidateNotExist class with a slight different implementation (using switch) and named it as ValidateNotExistImproved.

But I think, the entire code can be improved further. Can someone help me figure out what mistakes am I doing and how can I improve it?

The entire code is provided with a little explanation for every class.

NotExistErrorCodeConfig


Error codes configuration.

public static class NotExistErrorCodeConfig
{
    public const string Country = "COUNTRY_NOT_EXIST";
    public const string State = "STATE_NOT_EXIST";
    public const string City = "CITY_NOT_EXIST";
}


EntityTypeEnum


Enum Configuration, this is being used in the ValidateNotExistImproved class.

public enum EntityTypeEnum
{
    Country = 1,
    State = 2,
    City = 3
}


ExistsHelper


The actual logic is implemented and it has dedicated functions.

public static class ExistsHelper
{
    public static bool Country(int id)
    {
        return (id > 10) ? true : false;
    }

    public static bool State(int id)
    {
        return (id > 10) ? true : false;
    }

    public static bool City(int id)
    {
        return (id > 10) ? true : false;
    }
}


ValidateNotExist


A helper class, the implementation calls this class. This class is responsible to check the value and return the relevant error message.

```
public static class ValidateNotExist
{
public static string Country(int id)
{
return (!ExistsHelper.Country(id)) ? NotExistErrorCodeConfig.Country : string.Empty;
}

public static string State(int id)
{
return (!ExistsHelper.State(id)) ? NotExistErrorCodeConfig.State : string.Empty;
}

public static string City(int id)
{
return (!ExistsHelper.City(id)) ? NotExistErrorCodeConfig.City : string.E

Solution

public static class ExistsHelper
{
    public static bool Country(int id)
    {
        return (id > 10) ? true : false;
    }

    public static bool State(int id)
    {
        return (id > 10) ? true : false;
    }

    public static bool City(int id)
    {
        return (id > 10) ? true : false;
    }
}


The usage of a ternary here is senseless, you can just return the evaluated condition like so

public static class ExistsHelper
{
    public static bool Country(int id)
    {
        return (id > 10);
    }

    public static bool State(int id)
    {
        return (id > 10);
    }

    public static bool City(int id)
    {
        return (id > 10);
    }
}


but looking at it, it seems it doesn't matter what "type" you are validating because they all have the same condition. You could simply use only one method and extract the magic number 10 into a meaningful constant.

ValidateNotExistImproved

Here you are using a ternary as well which isn't needed because a simple if statement would be enough and more readable like so

public static string Validate(EntityTypeEnum type, int id)
{
    switch (type)
    {
        case EntityTypeEnum.Country:
            if (!ExistsHelper.Country(id))
            {
                return NotExistErrorCodeConfig.Country;
            }
            break;

        case EntityTypeEnum.State:
            if (!ExistsHelper.State(id))
            {
                return NotExistErrorCodeConfig.State;
            }
            break;

        case EntityTypeEnum.City:
            if (!ExistsHelper.City(id))
            {
                return NotExistErrorCodeConfig.City ;
            }
            break;
    }

    return string.Empty;
}

Code Snippets

public static class ExistsHelper
{
    public static bool Country(int id)
    {
        return (id > 10) ? true : false;
    }

    public static bool State(int id)
    {
        return (id > 10) ? true : false;
    }

    public static bool City(int id)
    {
        return (id > 10) ? true : false;
    }
}
public static class ExistsHelper
{
    public static bool Country(int id)
    {
        return (id > 10);
    }

    public static bool State(int id)
    {
        return (id > 10);
    }

    public static bool City(int id)
    {
        return (id > 10);
    }
}
public static string Validate(EntityTypeEnum type, int id)
{
    switch (type)
    {
        case EntityTypeEnum.Country:
            if (!ExistsHelper.Country(id))
            {
                return NotExistErrorCodeConfig.Country;
            }
            break;

        case EntityTypeEnum.State:
            if (!ExistsHelper.State(id))
            {
                return NotExistErrorCodeConfig.State;
            }
            break;

        case EntityTypeEnum.City:
            if (!ExistsHelper.City(id))
            {
                return NotExistErrorCodeConfig.City ;
            }
            break;
    }

    return string.Empty;
}

Context

StackExchange Code Review Q#134835, answer score: 4

Revisions (0)

No revisions yet.