Problem
We have a winforms application with a lot of grid views. These grids have some basic layout functions (text color, font weight, font style, ….). I need to make sure that other developers can make customizations to the layout of the grids without touching the base code. So this is the solution i came up with.
I created a generic meta object that can house the different default properties and the customizable properties
public class MetaObject<TModel>
{
public int ColumnOrder { get; }
public bool Editable { get; }
public Func<TModel,string> TextFormat { get; set; }
public Func<TModel,string> FontStyle { get; set; }
public Func<TModel,string> ColorCode { get; set; }
}
the customizable properties are funcs so the ‘user’ can insert some custom logic.
I created a new object deriving from List so the data and the metaData are in the same object.
public class ListWithMetaData<TModel> : List<TModel>, IListWithMetaData<TModel>
{
private Dictionary<string, MetaObject<TModel>> _metaData;
public Dictionary<string, MetaObject<TModel>> MetaData
{
get => _metaData ?? ( _metaData = new Dictionary<string, MetaObject<TModel>>());
set => _metaData = value;
}
public MetaObject<TModel> GetMetaDataFor(Expression<Func<TModel, object>> property)
{
if (property.NodeType != ExpressionType.Lambda)
return null;
var lambda = (LambdaExpression) property;
var memberExpression = ExtractMemberExpression(lambda.Body);
if (memberExpression == null)
{
throw new ArgumentException("Selector must be member access expression", nameof(property));
}
if (memberExpression.Member.DeclaringType == null)
{
throw new InvalidOperationException("Property does not have declaring type");
}
var memberName = memberExpression.Member.Name;
return _metaData.ContainsKey(memberName) ? _metaData[memberName] : null;
}
private MemberExpression ExtractMemberExpression(Expression expression)
{
if (expression.NodeType == ExpressionType.MemberAccess)
{
return ((MemberExpression) expression);
}
if (expression.NodeType == ExpressionType.Convert)
{
var operand = ((UnaryExpression) expression).Operand;
return ExtractMemberExpression(operand);
}
return null;
}
}
the metaData is stored in a Dictionary per Property.
usage of this looks like this
static void Main(string[] args)
{
var movements = GetMovements();
foreach (var movement in movements)
{
var numberMeta = movements.GetMetaDataFor(x => x.Number);
if (numberMeta != null)
{
var colorCode = numberMeta.ColorCode?.Invoke(movement) ?? "";
var textFormat = numberMeta.TextFormat?.Invoke(movement) ?? "";
Console.WriteLine(
$"Textformat:t {textFormat} t=> {movement.Number.ToString(textFormat)}t Color t {colorCode}");
}
}
Console.ReadLine();
}
private static ListWithMetaData<Movement> GetMovements()
{
var items = new ListWithMetaData<Movement>();
items.MetaData.Add(nameof(Movement.Number), new MetaObject<Movement>
{
TextFormat = m => m.Number % 2 == 0 ? "###" : "000.00",
ColorCode = m => MovementColorPicker.GetRemoteColorCode(m.Pk)
});
for (int i = 0; i < 20; i++)
{
var item = new Movement
{
Pk = $"PK-{i}",
Text = $"Text-{i}",
Number = i,
Date = DateTime.Today.AddDays(i)
};
items.Add(item);
}
return items;
}
this frame does the work it needs to do, but I am not sure if its the most ‘user friendly’ way of allowing customizations.
Any advice is appreciated.
Solution
Consistency
items.MetaData.Add(nameof(Movement.Number), new MetaObject<Movement>{});
public MetaObject<TModel> GetMetaDataFor(Expression<Func<TModel, object>> property)
I would suggest sticking with a single approach. Either use a string or an expression in both cases. Currently, consumers are allowed to use any string to add to the metadata list, but when retrieving items from the list, they are bound to existing properties of TModel.
The better approach depends on whether you want to allow columns for calculated values or not (i.e. data that is not an existing property of TModel).
If not, then stick with using Expression in both cases; so that you only allow existing properties to be referenced. Note that you can still use a string key internally, but don’t expose that to your consumer. Your consumer should only use expression to set the values.
If yes, then stick with strings. This allows a consumer to either use an existing property (which is converted to string), or a custom string of their choosing (for a calculated value).
Note that you can still provide overloads so both approaches are allowed:
- The first method takes a string, which can be called directly by the consumer.
- There is an second (overloaded) method that takes an expression, but internally converts it to a string and then calls the first method.
This can be implemented both on the Get and Add methods.
If you’re allowing calculated values, I would also suggest adding a CellValue property to your metadata, so your consumer can define the content of the cell.
public class MetaObject<TModel>
{
public Func<TModel,string> CellValue { get; set; }
}
Which allows things like:
items.MetaData.Add("MyCustomColumn", new MetaObject<Movement>
{
TextFormat = m => m.Number % 2 == 0 ? "###" : "000.00",
ColorCode = m => MovementColorPicker.GetRemoteColorCode(m.Pk),
CellValue = m => $"Number {m.Number} from {m.SourceName}"
})
While still allowing to use property values that are not calculated:
items.MetaData.Add(m => m.Number, new MetaObject<Movement>
{
TextFormat = m => m.Number % 2 == 0 ? "###" : "000.00",
ColorCode = m => MovementColorPicker.GetRemoteColorCode(m.Pk),
CellValue = m => m.Number
})
Comment
For the purpose of this snippet, ignoreitems.MetaData.Add()and its signature. You will want to create specific methods for adding/retrieving items, but I omitted this here because it’s not the focus of what I’m currently discussing.Practically, you should substitute
items.MetaData.Add()with a fitting method that you create.
Naming
I don’t like the name of MetaObject. A more appropriate name would be ColumnSettings.
The issue here is that you seem to misuse “metadata” in general. The metadata of an object still pertains to that object, but not its primary purpose.
A simple example here is the metadata of an MP3 file, such as the title of the track, album, artist, and genre. It’s not part of the primary purpose (the sound data itself), but it’s additional information that a consumer (media player) may want to use (but is not required to use).
Notice that this metadata does not contain settings for the consumer of the data (media player), e.g. what volume to play the song at, or what equalizer settings to use.
These settings (volume/equalizer) should not be unique for a particular song; the user will want those settings to persist between all the songs he’s listening to. Furthermore, the user may want to listen to the same song on a different volume level (e.g. during the day and at night). Attaching a volume setting to the song means that you overridde the user’s current volume setting. A media player that overrides the user’s explicit wishes will not be liked by many users.
This is why your “metadata” isn’t actually metadata for TModel. It’s actually metadata for your Grid.
Fixing the naming is one thing, but we also need to fix the underlying structure that you’re using; you’re not adding the “metadata” at the right level. That is discussed in depth in the next Chapter.
Inheritance abuse
public class ListWithMetaData<TModel> : List<TModel>, IListWithMetaData<TModel>
You’re extending a list of entities with the column properties, but these things are two very different things.
From the code, I surmise that you’re thinking that a list of entities is fixed, at which point metadata is added, which is then sent to the grid.
But what if your list of entities is paginated? This means that you will get multiple lists of entities (one at a time); and you’d be stuck adding the same metadata to every list. This also means that you can’t e.g. use a DataSource object, because you need to wrap every retrieved list of entities in a method that provides the metadata.
These things need to be separate. You can take the example of how the Winforms Grid works. You can set the Column properties at design time, without needing a list of entities to display.
You’ll want to do the same thing, so that a consumer can separate the design logic from the loading logic. There are two options here, depending on your preference.
Preference 1 – Separating the design from the data.
FancyGrid fancyGrid = new FancyGrid();
fancyGrid.ColumnSettings = new List<ColumnSettings<MyModel>>() { .. };
fancyGrid.Items = new List<MyModel>();
This completely separates the design of the grid from the data that it’s supposed to display. You only need to set the ColumnSettings once, and are then able to reload data whenever you want to.
If you take this approach, then it makes sense to make FancyGrid generic. This prevents consumers from using a different generic type for ColumnSettings and Items:
FancyGrid<MyModel> fancyGrid = new FancyGrid<MyModel>();
//compile time error
fancyGrid.ColumnSettings = new List<ColumnSettings<MyOtherModel>>() { .. };
//works
fancyGrid.ColumnSettings = new List<ColumnSettings<MyModel>>() { .. };
//compile time error
fancyGrid.Items = new List<MyOtherModel>();
//works
fancyGrid.Items = new List<MyModel>();
Preference 2 – Setting data and design in a single call.
This can be done in many ways. The main point that I’m stressing here that you don’t need the inheritance.
You could simply pass two separate parameters:
public void LoadGridItems<TModel>(IEnumerable<ColumnSettings<TModel>> columnSettings, IEnumerable<TModel> data)
Or you can merge them in a class if you really want to. However, you should use composition over inheritance:
public class FancyGridData<TModel>
{
public IEnumerable<ColumnSettings<TModel>> ColumnSettings { get; set; }
public IEnumerable<TModel> Data { get; set; }
}
Note that this is a replacement for ListWithMetaData<TModel> in your initial code. I dislike the initial name, but explaining why required me to first explain the inheritance issue.
Whether you use this class or not is a matter of preference. I don’t think it’s warranted; but your initial code suggests to me that you might prefer this abstraction to simplify the consumer call.