locked
How to remove the duplicate code RRS feed

  • Question

  • User574857395 posted

    Hi,

    I have the following code:

        public static string GetLeftPaddedString(string value, int paddingWidth, char padChar)
        {
          StringBuilder paddedString = new StringBuilder(value);
          int totalPaddingWidth = paddingWidth - value.Length;
          if (totalPaddingWidth > 0)
          {
            paddedString.Clear();
            paddedString.Append(value.PadLeft(totalPaddingWidth, padChar));
          }
          return paddedString.ToString();
        }
    
        public static string GetRightPaddedString(string value, int paddingWidth, char padChar)
        {
          StringBuilder paddedString = new StringBuilder(value);
          int totalPaddingWidth = paddingWidth - value.Length;
          if (totalPaddingWidth > 0)
          {
            paddedString.Clear();
            paddedString.Append(value.PadRight(totalPaddingWidth, padChar));
          }
          return paddedString.ToString();
        }

    They have duplicate code. I'd like there to be one method that is able to take the type of padding (left or right) and do it appropriately. I know this can be done. I'm having trouble figuring it out. I'd like to be able to do this without over engineering too much.

    Saturday, April 25, 2015 6:20 AM

Answers

  • User281315223 posted

    Although the code looks pretty, a static dictionary seems a bit like overkill for a function like this, however it's entirely up to you.

    If there were just two scenarios (i.e. left and right), I wouldn't worry about over-complicating it and storing a dictionary of expressions and instead just map the enum to the appropriate operation :

    public static string GetPaddedString(string value, int totalLength, char padChar, PadType padType)
    {
          StringBuilder paddedString = new StringBuilder(value);
          int totalPaddingWidth = totalLength - value.Length;
          //if padding needs to be added, add the specific type of padding
          if (totalPaddingWidth > 0)
          {
              paddedString.Clear();
              switch(padType)
              {
                   case PadType.Left:
                       paddedString.Append(value.PadLeft(totalPaddingWidth, padChar));
                   break;
                   case PadType.Right:
                       paddedString.Append(value.PadRight(totalPaddingWidth, padChar));
                   break;
              }  
          }
          return paddedString.ToString();
    }

    Just my opinion though.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Saturday, April 25, 2015 8:31 AM

All replies

  • User281315223 posted

    It just depends what you want to use as a parameter to pass in to indicate if it should be a left or right padding :

    public static string GetPaddedString(string value, int paddingWidth, char padChar, string direction)
    {
          StringBuilder paddedString = new StringBuilder(value);
          int totalPaddingWidth = paddingWidth - value.Length;
          if (totalPaddingWidth > 0)
          {
               paddedString.Clear();
               switch(direction)
               {
                    case "Left":
                    default:
                        paddedString.Append(value.PadRight(totalPaddingWidth, padChar));
    
                    break;
                    case "Right":
                        paddedString.Append(value.PadLeft(totalPaddingWidth, padChar));
                    break;
               }
          }
          return paddedString.ToString();
    }

    This would simply allow you to call it through :

    GetPaddedString("Test", 8, ' ', "Left")

    or :

    GetPaddedString("Test", 8, ' ', "Right")

    This obviously just an example as you could use several ways to handle this such as creating an Enum which might make the code a bit more readable, but the same idea would hold true.

    Saturday, April 25, 2015 8:14 AM
  • User574857395 posted

    I was trying to avoid the string passing of direction as well. I rather shift the choosing of the direction to the caller and make this method able to handle whichever direction the caller chooses.

    I've come up with this cool code

        public enum PadType { Left, Right }
        //create a dictionary that encompasses methods to perform left or right padding
        private static Dictionary<PadType, Func<string, int, char, string>> PaddingMethods { get; set; }
    
        //intialize the PaddingMethods in a static constructor to ensure that this is only initialized once
        static Utils()
        {
          //initialize the dictionary with the operations to perform when encountering left or right padding
          PaddingMethods = new Dictionary<PadType, Func<string, int, char, string>>() 
          {
            { PadType.Left, (padString, padWidth, paddingChar) => { return padString.PadLeft(padWidth, paddingChar); } },
            { PadType.Right, (padString, padWidth, paddingChar) => { return padString.PadRight(padWidth, paddingChar); } }
          };
        }
    
        //gets a padded string equal to the total length
        public static string GetPaddedString(string value, int totalLength, char padChar, PadType padType)
        {
          StringBuilder paddedString = new StringBuilder(value);
          int totalPaddingWidth = totalLength - value.Length;
          //if padding needs to be added, add the specific type of padding
          if (totalPaddingWidth > 0)
          {
            paddedString.Clear();
            paddedString.Append(PaddingMethods[padType](value, totalLength, padChar));
          }
    
          return paddedString.ToString();
        }

    Saturday, April 25, 2015 8:22 AM
  • User281315223 posted

    Although the code looks pretty, a static dictionary seems a bit like overkill for a function like this, however it's entirely up to you.

    If there were just two scenarios (i.e. left and right), I wouldn't worry about over-complicating it and storing a dictionary of expressions and instead just map the enum to the appropriate operation :

    public static string GetPaddedString(string value, int totalLength, char padChar, PadType padType)
    {
          StringBuilder paddedString = new StringBuilder(value);
          int totalPaddingWidth = totalLength - value.Length;
          //if padding needs to be added, add the specific type of padding
          if (totalPaddingWidth > 0)
          {
              paddedString.Clear();
              switch(padType)
              {
                   case PadType.Left:
                       paddedString.Append(value.PadLeft(totalPaddingWidth, padChar));
                   break;
                   case PadType.Right:
                       paddedString.Append(value.PadRight(totalPaddingWidth, padChar));
                   break;
              }  
          }
          return paddedString.ToString();
    }

    Just my opinion though.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Saturday, April 25, 2015 8:31 AM
  • User574857395 posted

    Thanks. It does seem a bit of an overkill now that I look at it. The possibility for expansion isn't that great and I don't think there needs to be that sort of complication now that you mention it.

    Sunday, April 26, 2015 4:54 AM
  • User-1910946339 posted

    I might be missing something here .... but it won't be the first time.

    It looks to me that you are just rewriting the String.PadLeft method.

    public static string GetLeftPaddedString(string value, int paddingWidth, char padChar)
    {
       return value.PadLeft(paddingWidth, padChar);
    }

    There is also a String.PadRight method.

    The best way to de-duplicate is to delete the code altogether!

    Sunday, April 26, 2015 8:02 PM
  • User-1910946339 posted

    As predicted, I got it wrong.

    GetLeftPaddedString is a bit of an odd bird.  I did some tests and got

    GetLeftPaddedString("Hello", 2, '*') == "Hello"
    GetLeftPaddedString("Hello", 5, '*') == "Hello"
    GetLeftPaddedString("Hello", 10, '*') == "Hello"
    GetLeftPaddedString("Hello", 12, '*') == "**Hello"

    For a string of length n.  If paddingWidth is less that or equal to 2*n then the original string is returned.  If paddingWidth is greater than 2*n then (paddingWidth - 2*n) characters are appended to the front of the string.

    This is a bit odd.  Is it really what you are trying to do?

    Sunday, April 26, 2015 8:22 PM
  • User-1910946339 posted

    If you really want that interesting functionality then

    public static string GetLeftPaddedString(string value, int paddingWidth, char padChar)
    {
       return value.PadLeft(Math.Max(0, paddingWidth - value.Length), padChar);
    }

    Sunday, April 26, 2015 8:38 PM