Modify multiple string fields

Question

I have the following code:

  class SearchCriteria
  {
        public string Name { get; set; }
        public string Email { get; set; }
        public string Company { get; set; }
        // ... around 20 fields follow

        public void Trim()
        {
            if( ! String.IsNullOrEmpty( Name ) )
            {
                 Name = Name.Trim();
            }

            if( ! String.IsNullOrEmpty( Email ) )
            {
                Email = Email.Trim();
            }

            // ... repeat for all 20 fields in the class.
        }
   }

I want to write one function that will properly trim the fields, something like:

public void Trim()
{
    Trim( Name );
    Trim( Email );
    // ...
}

private static void Trim( ref string field )
{
    if( ! String.IsNullOrEmpty( field ) )
    {
        field = field.Trim();
    } 
}

Of course, this is not permitted in C#. One option I have is to write a helper and use reflection. Is there another way I can achieve this (reflecting on so many properties will deffinitely have a performance hit on that particular scenario and I can't afford that)?

Maybe you could trim the strings in their respective property setters? — Frédéric Hamidi, Apr 23 '15 at 08:54
I would probably redesign the whole class to use a HashMap for saving 20 different String values since they seem a List of Optional attributes. Then you have one PUT Method, which could do the trimming. — Falco, Apr 23 '15 at 10:02
@Falco: how do you know that all are strings? There could be an `DateTime DayOfBirth`. Using a `List` would be too abstract since you cannot say: give me `SearchCriteria.Email` anymore. And if you use a `Dictionary` you have to know the key. — Tim Schmelter, Apr 23 '15 at 14:27
@TimSchmelter You can create static constants for the key-names of the Dictionary. Or an Enum and have a dictionary this will clean up the class a lot. -> And I'm sure there are only String fields, because he wants to call trim() on all of them. You cannot trim a date ;-) — Falco, Apr 24 '15 at 09:49
What i wanted to say is that it's not uncommon to have 20 properties and that it's normally not better to use a `List` for them. You save few lines but you get worse code because all have to be strings, you cannot change it in future(f.e. change `string Company` to `Company Company`). You have to loop them or know the keys somehow, the class isn't readable or maintainable anymore for other developers. Basically are you replacing OOP concepts with strings. — Tim Schmelter, Apr 24 '15 at 09:55

Tim Schmelter · Accepted Answer · 2015-04-23T09:06:56.500

If you already have the code, what are you asking? It's readable and efficient. But maybe it would be better to let the properties already trim the passed value in the first place.

class SearchCriteria
{
    private string _Name;
    public string Name
    {
        get { return _Name; }
        set { _Name = value == null ? null : value.Trim(); }
    }

    private string _Email;
    public string Email
    {
        get { return _Email; }
        set { _Email = value == null ? null : value.Trim(); }

    }

    private string _Company;
    public string Company
    {
        get { return _Company; }
        set { _Company = value == null ? null : value.Trim(); }

    }

    // ... around 20 fields follow
}

Even if you could use a reflection approach. Consider that this code is always difficult to understand and to maintain. And it will silently trim properties even if they should not be trimmed. For example if another developer extends this class.

ASh · Answer 2 · 2015-04-23T12:03:24.960

public void Trim()
{
    Name = Trim( Name );
    Email = Trim( Email );
    // ...
}

private string Trim(string field )
{
    if( ! String.IsNullOrEmpty( field ) )
        field = field.Trim();
    return field;
}

EDIT:

try also to apply Trim fuction in setters of properties

class SearchCriteria
{    
    private string Trim(string field)
    {
        if( ! String.IsNullOrEmpty( field ) )
            field = field.Trim();
        return field;
    }

    private string _name;
    public string Name
    {
        get { return _name; }
        set { _name = Trim(value); }
    }

    private string _email;
    public string Email
    {
        get { return _email; }
        set { _email = Trim(value); }

    }

    // ... other string properties
    // no public void Trim() method
}

Eric · Answer 3 · 2015-04-23T09:09:20.133

3

Seems overkill .. saved the time in Trim(), wasted the time in field declaration

class SearchCriteria
{
    private Dictionary<string, string> _internalValues = new Dictionary<string, string>();
    public string Name { get { return _internalValues.ContainsKey("Name") ? _internalValues["Name"] : null; } set { _internalValues["Name"] = value; } }
    ....

    public void Trim()
    {
        foreach (var entry in _internalValues)
        {
            if (!string.IsNullOrEmpty(entry.Value)) _internalValues[entry.Key] = entry.Value.Trim();
        }
    }
}

edited Apr 23 '15 at 09:09

answered Apr 23 '15 at 09:01

Eric

5,675
16
24

If there are no legacy reasons why the public interface of the class has to stay the same, I would scrap the whole accessors and simply do SearchCriteria.get( SCEnum.Email ); – Falco Apr 24 '15 at 09:52

score 3 · Answer 4 · answered Apr 23 '15 at 09:10

I prefer this style, the duplication is unavoidable to maintain readability, but this will save some screen space

class SearchCriteria
{
   public string Name { get; set; }
   public string Email { get; set; }
   public string Company { get; set; }


   public void Trim()
   {
        if(!String.IsNullOrEmpty(Name)) Name = Name.Trim();
        if(!String.IsNullOrEmpty(Email)) Email = Email.Trim();
        if(!String.IsNullOrEmpty(Company)) Company = Company.Trim();
   }
}

void Main()
{
    var criteria = new SearchCriteria();
    criteria.Email = "thing ";
    Console.WriteLine(criteria.Email.Length);
    criteria.Trim();
    Console.WriteLine(criteria.Email);
    Console.WriteLine(criteria.Email.Length);
}

score 3 · Answer 5 · answered Apr 23 '15 at 11:37

If you weren't using auto properties you could just use a ref. I agree it is by no means optimal.

class SearchCriteria
{
    private string _name;
    public string Name { get { return _name; } set { _name = value; }}
    public string Email { get; set; }
    public string Company { get; set; }
    // ... around 20 fields follow

    void Trim(ref string str)
    {
        if (!String.IsNullOrEmpty(str))
        {
            str = str.Trim();
        }
    }

    public void Trim()
    {
        Trim(ref _name);

        // ... repeat for all 20 fields in the class.
    }
}

Kami · Answer 6 · 2015-04-23T09:18:44.090

You can amend your code as follows to

public void Trim()
{
    Name = Trim(Name);
    Email = Trim(Email);
    // ...
}

private static void Trim(string field)
{
    if( ! String.IsNullOrWhiteSpace( field ) )
    {
        field = field.Trim();
    }

    return field;
}

You cannot pass a property by reference, and the method String.IsNullOrEmpty() will consider white-space as non-empty, so I have used String.IsNullOrWhiteSpace().

score 2 · Answer 7 · answered Apr 23 '15 at 09:09

Just out of completion. I don't know if this is a good way. But you can use as Tim Schmelter said reflection. But as he also pointed out the code is harder to maintain and if someone extend it might be problems. But here is an example of how you could do it:

class SearchCriteria
{
   public string Name { get; set; }
   public string Email { get; set; }
   public string Company { get; set; }
   // ... around 20 fields follow

   public void Trim()
   {
       typeof(SearchCriteria).GetProperties()
             .Where (w =>w.PropertyType==typeof(string))
             .ToList().ForEach(f=>
             {
                 var value=f.GetValue(this);
                 if(value!=null && !string.IsNullOrEmpty(value.ToString()))
                 {
                    f.SetValue(this,value.ToString().Trim(),null);
                 }
             });
   }
}

score 2 · Answer 8 · edited May 23 '17 at 12:21

Reflection will obviously be not the most performant solution, but with some modifications and caching it still can be used.

Here is the reflection helper that will allow you to create collection of mutator delegates and cache it inside of your class:

public static class ReflectionHelper
{
    public static IEnumerable<PropertyInfo> GetPropertiesOfType<THolder, TPropType>()
    {
        return typeof(THolder).GetPropertiesOfType(typeof(TPropType));
    }


    public static IEnumerable<PropertyInfo> GetPropertiesOfType(this Type holderType, Type propType)
    {
        if (holderType == null)
            throw new ArgumentNullException("holderType");
        if (propType == null)
            throw new ArgumentNullException("propType");

        return holderType
            .GetProperties()
            .Where(prop =>
                prop.PropertyType == propType); 
    }


    public static IEnumerable<Action<Func<TPropType, TPropType>>> CreateMutators<THolder, TPropType>(THolder holder)
    {
        if (holder == null)
            throw new ArgumentNullException("holder");

        return holder.GetType()
            .GetPropertiesOfType(typeof(TPropType))
            .Select(prop =>
                new
                {
                    getDelegate = (Func<TPropType>)Func.CreateDelegate(
                         typeof(Func<TPropType>), 
                         holder, 
                         prop.GetGetMethod()),
                    setDelegate = (Action<TPropType>)Action.CreateDelegate(
                         typeof(Action<TPropType>), 
                         holder, 
                         prop.GetSetMethod())
                })
            .Select(accessor =>
                (Action<Func<TPropType, TPropType>>)((mutate) =>
                {
                    var original = accessor.getDelegate();
                    var mutated = mutate(original);
                    accessor.setDelegate(mutated);
                }))
            .ToArray();
    }
}

Class code - you cache the mutators and use them inside the Trim method:

class SearchCriteria
{
    public SearchCriteria()
    {
        this.Name = "adsfasd     ";
        this.Email = "       adsfasd     ";
        this.Company = "  asdf   adsfasd     ";

        this.stringMutators = ReflectionHelper.CreateMutators<SearchCriteria, String>(this);
    }

    public string Name { get; set; }
    public string Email { get; set; }
    public string Company { get; set; }
    // ... around 20 fields follow

    private IEnumerable<Action<Func<String, String>>> stringMutators;


    private String TrimMutate(String value)
    {
        if (String.IsNullOrEmpty(value))
            return value;

        return value.Trim();
    }

    public void Trim()
    {
        foreach (var mutator in this.stringMutators)
        {
            mutator(this.TrimMutate);
        }
    }

    public override string ToString()
    {
        return String.Format("Name = |{0}|, Email = |{1}|, Company = |{2}|",
            this.Name,
            this.Email,
            this.Company);
    }
}

Main code:

            var criteria = new SearchCriteria();

            Console.WriteLine("Before trim:");
            Console.WriteLine(criteria);


            Console.WriteLine("After trim:");
            criteria.Trim();
            Console.WriteLine(criteria);

P.S.: However it is not very direct or clear solution, so I'd recommend to go with "smart" setter(getters) as it is described in other answers. Or, perhaps, you can try some Aspect Oriented Programming approach.

Modify multiple string fields

8 Answers8