String modification application

Question

Below is working code of a semi complete program. Its purpose is to take an input string of any type and modify it based on rules defined for each type. So in this example I pass it a string in CSV format and modify some of the fields.

My questions are

Can the use of interfaces be improved?
How can I handle different string modification patterns (I currently make it work by using IStringModificationPattern - but I want to extend this to other types like HTML)?
What design changes/ additions can I make to integrate the string modification pattern into a configuration class (i.e. using an XML file based settings)?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace StringModificationTest
{
    class Program
    {
        static void Main(string[] args)
        {
            //usage example
            //
            string inputText = "CSV,field1,field2,field3,field4,field5,field6,field7,,,,";

            StringModifier SM = new StringModifier();

            string outputText = SM.ModifyString(inputText);

        }
    }

StringModifier class

    public class StringModifier
    {
        private List<IStringType> RegisteredTypes = new List<IStringType>();
        private IStringModificationPattern ModificationPattern;

        public StringModifier()
        {
            RegisterTypes();
        }

        private void RegisterTypes()
        {
            RegisteredTypes = TypeFactory.GetTypeList();
        }

        public string ModifyString(string str)
        {
            ///sets the message and sets the Modification pattern
            IString detectedStr = DetectStringType(str);

            ///returns the modified string
            return detectedStr.Modify(ModificationPattern);
        }

        private IString DetectStringType(string str)
        {
            IString msg = new NullString(str);

            for (int i = 0; i < RegisteredTypes.Count; i++)
            {
                if (str.StartsWith(RegisteredTypes[i].start))
                {
                    //set string type
                    msg = RegisteredTypes[i].getNewInstance(str);

                    //set modification pattern
                    ModificationPattern = RegisteredTypes[i].MODPattern;
                }

            }

            return msg;

        }

    }

TypeFactory static class

    internal static class TypeFactory
    {
        public static List<IStringType> GetTypeList()
        {
            List<IStringType> types = new List<IStringType>();
            types.AddRange(from assembly in AppDomain.CurrentDomain.GetAssemblies()
                           from t in assembly.GetTypes()
                           where t.IsClass && t.GetInterfaces().Contains(typeof(IStringType))
                           select Activator.CreateInstance(t) as IStringType);

            return types;
        }
    }

IStringType interface

    public interface IStringType
    {
        string start { get;  }
        IString getNewInstance(string STR);
        IStringModificationPattern MODPattern { get; }
    }

IStringType Implementations

    public class CSVType : IStringType
    {
        public string start { get { return @"CSV,"; } }

        public IStringModificationPattern MODPattern { get { return new CSV_StringModificationPattern(); } }

        public IString getNewInstance(string str)
        {
            return (IString)(new CSVString(str));
        }
    }

    public class HTMLStringType : IStringType
    {
        public string start { get { return @"<HTML>"; } }

        public IStringModificationPattern MODPattern { get { return new HTML_StringModificationPattern(); } }

        public IString getNewInstance(string str)
        {
            return (IString)(new HTMLString(str));
        }

    }

IString interface

    public interface IString
    {
        string STR { get; }
        string Modify(IStringModificationPattern MODPattern);

    }

IString implementations

    public class CSVString : IString
    {
        public string STR { get; set; }
        public List<string> CSVFields { get; set; }

        public CSVString(string str)
        {
            STR = str;
            CSVFields = new List<string>();
            CSVFields.AddRange(str.Split(','));
        }

        public string Modify(IStringModificationPattern MODPattern)
        {
            string output = STR;

            foreach (int i in MODPattern.location)
            {
                CSVFields[i] = "BLAH";
            }

            //rebuild string
            System.Text.StringBuilder sb = new System.Text.StringBuilder();
            for (int i = 0; i < CSVFields.Count; i++)
            {
                sb.Append(CSVFields[i]);
                if (i != CSVFields.Count - 1)
                    sb.Append(',');
            }
            output = sb.ToString();

            return output;
        }
    }

    public class HTMLString : IString
    {
        public string STR { get; set; }
        public HTMLString(string str)
        {
            STR = str;
        }

        public string Modify(IStringModificationPattern MODPattern)
        {
            string output = STR;



            return output;
        }

    }

    public class NullString : IString
    {
        public string STR { get; set; }
        public NullString(string str)
        {
            STR = str;
        }

        public string Modify(IStringModificationPattern MODPattern)
        {
            string output = STR;
            //Do nothing
            return output;
        }

    }

IStringModificationPattern interface

    public interface IStringModificationPattern
    {
        List<int> location { get; set; }
    }

IStringModificationPattern implementations

    public class CSV_StringModificationPattern : IStringModificationPattern
    {
        //defines the fields that need to be modified
        public List<int> location { get; set; }

        public CSV_StringModificationPattern()
        {
            location = new List<int>();
            location.Add(2);
            location.Add(5);
        }
    }

    public class HTML_StringModificationPattern : IStringModificationPattern
    {
        public List<int> location { get; set; }
    }
}

EDIT: Revised Code taking into consideration feedback below

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace StringModificationTest2
{
    class Program
    {
        static void Main(string[] args)
        {
            //usage example
            //
            string inputText = "CSV,field1,field2,field3,field4,field5,field6,field7,,,,";

            StringModifier SM = new StringModifier();

            string outputText = SM.Modify(inputText);
        }
    }

    public class StringModifier
    {
        private readonly List<IStringModificationPattern> _registeredPatterns;
        private readonly StringPatternDetector Detector;

        public StringModifier()
        {
            _registeredPatterns = StringModificationPatternFactory.GetTypeList();
            Detector = new StringPatternDetector();
        }

        public string Modify(string inputValue)
        {
            //get the modification pattern that belongs to this string, and the string object
            IStringModificationPattern Modificationpattern = Detector.DetectPattern(this._registeredPatterns, inputValue);
            IString StringObject = Detector.StringObject;

            //execute correct modification algorithm on the input string
            return StringObject.Modify(Modificationpattern); ;
        }
    }

    public class StringPatternDetector
    {
        private IStringModificationPattern _pattern;

        public IString StringObject { get; private set; }

        public StringPatternDetector()
        {
            StringObject = new DEFAULTString(string.Empty);
        }

        public IStringModificationPattern DetectPattern(List<IStringModificationPattern> registeredPatterns, string value)
        {
            for (int i = 0; i < registeredPatterns.Count; i++)
            {
                if (value.StartsWith(registeredPatterns[i].start))
                {
                    _pattern = registeredPatterns[i];
                }
            }

            if (_pattern != null)
                StringObject = _pattern.getNewInstance(value);
            else
                StringObject = new DEFAULTString(value);

            return _pattern;
        }
    }


    internal static class StringModificationPatternFactory
    {
        public static List<IStringModificationPattern> GetTypeList()
        {
            List<IStringModificationPattern> types = new List<IStringModificationPattern>();
            types.AddRange(from assembly in AppDomain.CurrentDomain.GetAssemblies()
                           from t in assembly.GetTypes()
                           where t.IsClass && t.GetInterfaces().Contains(typeof(IStringModificationPattern))
                           select Activator.CreateInstance(t) as IStringModificationPattern);

            return types;
        }
    }



    public interface IString
    {
        string STR { get; }
        string Modify(IStringModificationPattern MODPattern);
    }

    public class CSVString : IString
    {
        public string STR { get; set; }
        public List<string> CSVFields { get; set; }

        public CSVString(string str)
        {
            STR = str;
            CSVFields = new List<string>();
            CSVFields.AddRange(str.Split(','));
        }

        public string Modify(IStringModificationPattern MODPattern)
        {
            string output = STR;

            foreach (int i in MODPattern.location)
            {
                CSVFields[i] = "BLAH";
            }

            //rebuild string
            System.Text.StringBuilder sb = new System.Text.StringBuilder();
            for (int i = 0; i < CSVFields.Count; i++)
            {
                sb.Append(CSVFields[i]);
                if (i != CSVFields.Count - 1)
                    sb.Append(',');
            }
            output = sb.ToString();

            return output;
        }
    }

    public class HTMLString : IString
    {
        public string STR { get; set; }
        public HTMLString(string str)
        {
            STR = str;
        }

        public string Modify(IStringModificationPattern MODPattern)
        {
            string output = STR;

            return output;
        }
    }
    public class DEFAULTString : IString
    {
        public string STR { get; set; }
        public DEFAULTString(string str)
        {
            STR = str;
        }

        public string Modify(IStringModificationPattern MODPattern)
        {
            string output = STR;
            //Do nothing
            return output;
        }
    }


    public interface IStringModificationPattern
    {
        string start { get; set; }
        List<int> location { get; set; }
        IString getNewInstance(string value);
    }
    public class CSV_StringModificationPattern : IStringModificationPattern
    {
        public string start { get { return @"CSV,"; } set { this.start = value; } }

        //defines the fields that need to be modified
        public List<int> location { get; set; }

        public CSV_StringModificationPattern()
        {
            location = new List<int>();
            location.Add(2);
            location.Add(5);
        }

        public IString getNewInstance(string value)
        {
            return (IString)(new CSVString(value));
        }
    }
}

Mathieu Guindon · Accepted Answer · 2014-04-30 05:26:08Z

The first word that comes to my mind, is over-engineering. This looks like a lot of trouble just to get to work with a CSV string, you turn a 10-liner into a party, happiness ensues and neighbors come too, and their friends also, and before you know it you've got 100 lines of code written, to accomplish the exact same thing.

This answer/review does not care about that.

StringModifier

The RegisteredTypes private field can be made readonly, and its naming doesn't match naming conventions for private types. I'd declare it like this:

private readonly List<IStringType> _registeredTypes;

And initialize it directly in the constructor, and remove the private RegisterTypes method altogether:

    public StringModifier()
    {
        _registeredTypes = TypeFactory.GetTypeList();
    }

While str is a well-understood short for string and I prefer str over @string, I think a better name for that parameter could be, simply, value - also I'd remove String from the method's name, it's in a class called StringModifier, it takes a string parameter and returns a string value. If Modify isn't clear enough, I don't know what is!

    public string Modify(string value)
    {
        var detectedString = DetectStringType(value);
        return detectedString.Modify(ModificationPattern);
    }

I don't think the comments add anything significant in the ModifyString method. You have descriptive method names, already doing the job of describing what the code does (and that's good!).

The DetectStringType method has opportunities for improvements - the RegisteredTypes needs only to be accessed once, and msg just seems like an arbitrary name - result makes more sense I find:

    private IString DetectStringType(string value)
    {
        IString result = new NullString(value);

        for (var i = 0; i < _registeredTypes.Count; i++)
        {
            var type = _registeredTypes[i];
            if (value.StartsWith(type.start))
            {
                result = type.GetNewInstance(value);
                ModificationPattern = type.MODPattern;
            }

        }

        return result;
    }

TypeFactory

The name is ambiguous, if not overly broad. StringTypeFactory would be a better name. This class is clearly infrastructure code. I don't see a need for it to be static though: by using it in StringModifier the way you are, you have tightly coupled the two classes, essentially making StringModifier responsible for finding all IStringType implementations in the current app domain... which seems like a lot to do for a StringModifier.

I'd move this infrastructure code to infrastructure level, and inject an IEnumerable<IStringType> into StringModifier's constructor:

    private readonly IEnumerable<IStringType> _registeredTypes;
    public StringModifier(IEnumerable<IStringType> types)
    {
        _registeredTypes = types;
    }

By infrastructure level, I mean this:

    var inputText = "CSV,field1,field2,field3,field4,field5,field6,field7,,,,";

    var factory = new StringTypeFactory();
    var modifier = new StringModifier(factory.GetTypeList());

    var outputText = modifier.Modify(inputText);

..and I know you're not going to like it. Because you probably want to be able to new up a StringModifier whenever you need to use one - my recommendation is to avoid newing up anything. With that many abstractions/interfaces, call me crazy but I'll say that you're missing an IStringModifier abstraction:

public interface IStringModifier
{
    string Modify(string value);
}

Now whenever you have a class that wants to use a StringModifier, you'll inject an IStringModifier through that class' constructor, assign a private readonly field, and use that field. This is called Dependency Injection, and it greatly helps decoupling code and making it more unit-testable. Creating objects is a concern on its own, that belongs to infrastructure code.

This hypothetical SomeClass consumes an IStringModifier and shows how constructor injection works:

public class SomeClass
{
    private readonly IStringModifier _modifier;

    public SomeClass(IStringModifier modifier)
    {
        _modifier = modifier;
    }

    public void Foo(string bar)
    {
        var modified = _modifier.Modify(bar);
        // ...
    }
}

But back to your code...

IStringType

The interface members don't follow casing conventions. Should look more like this:

public interface IStringType
{
    string Start { get; }
    IString GetNewInstance(string value);
    IStringModificationPattern Pattern { get; }
}

IString

The interface members don't follow casing conventions either:

public interface IString
{
    string Value { get; }
    string Modify(IStringModificationPattern pattern);
}

IStringType Implementations

The IStringModificationPattern property smells. All there is to it, is a getter that returns a new instance of a tightly coupled implementation of the modification pattern interface.

IString Implementations

The naming for NullString is dangerously ambiguous. VanillaString could probably work, ...and actually I can't think of any better names than that at the moment! ;)

The CsvString (PascalCase FTW!) could use an overloaded constructor that takes a custom separator. Handy when you have string values that may contain commas:

    public CsvString(string value) 
        : this(value, ',')
    { /* no op */ }

    public CsvString(string value, char separator)
    {
        Value = value;
        CsvFields = value.Split(separator);
    }

Now, your CsvString implementation has serious encapsulation issues:

    public string STR { get; set; }
    public List<string> CSVFields { get; set; }

Nothing prevents client code from overwriting all the hard work put into coming up with a value for STR. And nothing prevents client code from throwing away the CSVFields and replace it with another List<string> reference.

With auto-properties:

    public string Value { get; private set; }
    public IEnumerable<string> Fields { get; private set; }

Exposing Fields as an IEnumerable<string> rather than a List<string> allows client code to iterate the fields, but not to Add or Remove anything. In fact, the intent would be made even clearer with a readonly backing field for the fields' reference (no pun intended):

    private string _value;
    public string Value { get { return _value; } }

    private readonly IEnumerable<string> _fields;
    public string IEnumerable<string> Fields { get { return _fields; } }

The Principle of Least Knowledge tells us that if fields are meant to be added or removed from a CsvString, then CsvString should expose methods to do so (and then the backing field would probably be better of as an IList<string>), not letting its clients fiddle with it directly: the type of _fields is /should be of no concern to the client code.

I renamed CsvFields to just Fields, because it's a misleading name: there's no separator involved here, it's just ..fields :)

How can I handle different string modification patterns (I currently make it work by using IStringModificationPattern - but I want to extend this to other types like HTML)?

Before extending this API, I'd secure extensibility as a design concern, and address it with a SOLID response.

What design changes/ additions can I make to integrate the string modification pattern into a configuration class (i.e. using an XML file based settings)?

You could probably serialize your IStringModificationPattern implementations to XML.

Thanks a lot for your feedback. I added a section at the bottom of my original post with revised code based on your feedback. Not sure if I made use of all your recommendations correctly but i tried. My idea now was to work from the configuration settings and bringing that into the application. So the StringModificationpattern classes represent the settings I deserialize from XML (or take from GUI) and I would detect the string and instantiate my objects based on that. I kept the way the user or application calls this 'api' very simple using a wrapper that does everything behind the scenes. — erotavlas, Apr 30 '14 at 22:52
Additionally I can look into changing to the IEnumerable and encapsulate better my code, I was just trying to get it structured correctly so it works. — erotavlas, Apr 30 '14 at 22:56

Stack Exchange Network

current community

your communities

more stack exchange communities

String modification application

1 Answer 1

Your Answer

Not the answer you're looking for? Browse other questions tagged c# strings .net csv or ask your own question.

Hot Network Questions

String modification application

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c# strings .net csv or ask your own question.

Related

Hot Network Questions