debugcsharpMinor
Validation Error Handling - DRY Pattern Implementation
Viewed 0 times
handlingerrorvalidationdryimplementationpattern
Problem
The
Therefore, I have enhanced the
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.
EntityTypeEnum
Enum Configuration, this is being used in the ValidateNotExistImproved class.
ExistsHelper
The actual logic is implemented and it has dedicated functions.
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
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 sopublic 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.