3
\$\begingroup\$

In a recent project I needed a way to clone windows forms controls from a template which had predefined Size, Font, etc.

Since there is no Clone() method in the Control class, I created my own object that does that:

public class ControlTemplate<TSource>
    where TSource : Control
{
    private readonly List<Action<TSource>> _controlActions;

    public ControlTemplate()
    {
        _controlActions = new List<Action<TSource>>();
    }

    public ControlTemplate<TSource> WithArgument(Action<TSource> action)
    {
        if (action == null)
        {
            throw new ArgumentNullException(nameof(action));
        }
        _controlActions.Add(action);
        return this;
    }

    public TSource Clone()
    {
        TSource control = Activator.CreateInstance<TSource>();
        foreach (var action in _controlActions)
        {
            action.Invoke(control);
        }
        return control;
    }
}

It works somewhat like the Builder pattern, you can call WithArgument multiple times and all of the changes to the control will be saved in a list, to be later executed in the Clone method, on a fresh object in order to apply all of the changes.

The problem with this class is that it's not immutable and that's something really important here in order to have the same object every time you clone it.

If you have an instance of it stored somewhere that works as a template, outside classes can access the Clone method to create new object but they can also make modifications to the template using the WithArgument method, which will cause problems.

In order to fix that I made a "wrapper" class that only exposes the Clone method and operates with an instance of ControlTemplate internally which makes the template more immutable:

public class ControlTemplateCreator<TSource>
    where TSource : Control
{
    private readonly ControlTemplate<TSource> _sourceOfTemplate;

    public ControlTemplateCreator(ControlTemplate<TSource> sourceOfTemplate)
    {
        if (sourceOfTemplate == null)
        {
            throw new ArgumentNullException(nameof(sourceOfTemplate));
        }
        _sourceOfTemplate = sourceOfTemplate;
    }

    public TSource Clone()
    {
        return _sourceOfTemplate.Clone();
    }
}

Example usage would be:

public static class Settings
{
    private static readonly ControlTemplate<Label> _templateLabel;
    public static ControlTemplateCreator<Label> TemplateLabel { get; }

    static Settings()
    {
        _templateLabel = new ControlTemplate<Label>()
            .WithArgument(label => label.TextAlign = ContentAlignment.MiddleLeft)
            .WithArgument(label => label.AutoSize = false)
            .WithArgument(label => label.Size = new Size(135, 20))
            .WithArgument(label => label.Margin = new Padding(0))
            .WithArgument(label => label.Font = new Font("Aerial", 9));

        TemplateLabel = new ControlTemplateCreator<Label>(_templateLabel);
    }
}

public class Foo
{
    public void Bar()
    {
        Label clonedLabelFromTemplate = Settings.TemplateLabel.Clone();
    }
}

The code looks pretty good to me, it's extendable, you can create various templates from the same class, outside classes cant modify the original template.

Here are some of my questions:

  1. I wonder why isn't there a Clone method in the Control class are there any bad sides to cloning controls this way?

  2. Are there any flaws in the code?

  3. I'm not sure if WithArgument is a good name.

\$\endgroup\$
  • 2
    \$\begingroup\$ Sorry but I do not see the benefits compared to...a simple factory method (which might be less verbose...). Let's say even compared to a "raw" XAML/YAML serialization... \$\endgroup\$ – Adriano Repetti May 23 '17 at 15:45
  • \$\begingroup\$ This feels more flexible than a factory, if you don't agree, please write an answer, I will be more than happy to take a look at it :) \$\endgroup\$ – Denis May 23 '17 at 15:52
  • 1
    \$\begingroup\$ It's just a feeling, should see a real world use case. You write code in both cases then I don't see the advantages. Maybe if template comes from a text file with properties serialized as key/value pairs (XAML? YAML?) Not sure. \$\endgroup\$ – Adriano Repetti May 23 '17 at 17:12
2
\$\begingroup\$

I think you should remove Clone method from your builder. So you essentially would have:

interface ITemplateBuilder<TSource>
{
     ITemplateBuilder<TSource> WithArgument(...);
     ITemplate<TSource> Build();
}

where:

interface ITemplate<TSource>
{
     TSource Clone();
}

while it is similar to what you have, it conveys your intention in clearer way.

But overall I have to agree with Adriano, I think you are over-engineering. A simple factory will do the job and will probably take less code to write. However you can improve your implementation so it works more like a simplified version of WPF style, so that those actions you were saving up can be applied to any instance of Control. That, I think, would be much more useful. That you can't do with a factory.

 var style = new ControlStyle<Label>()
        .Setter(label => label.TextAlign = ContentAlignment.MiddleLeft)
        .......;
 style.Apply(_myLabel);
 style.Apply(_myCustomClassThatInheritsLabel); 
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.