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:
I wonder why isn't there a
Clone
method in theControl
class are there any bad sides to cloning controls this way?Are there any flaws in the code?
I'm not sure if
WithArgument
is a good name.
-
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– Adriano Repetti2017年05月23日 15:45:56 +00:00Commented May 23, 2017 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– Denis2017年05月23日 15:52:20 +00:00Commented May 23, 2017 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– Adriano Repetti2017年05月23日 17:12:51 +00:00Commented May 23, 2017 at 17:12
1 Answer 1
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);
Explore related questions
See similar questions with these tags.