patterncsharpMinor
Functional Html builder
Viewed 0 times
functionalhtmlbuilder
Problem
This is my third attempt to create a easy to use html builder because I wasn't really satisfied with the first one that wasn't extendable at all and even the one using dynamics wasn't much better. It was difficult to use due to everything being dynamic and had to be casted everywhere but what's worse it was hard to find the bug if something went wrong.
This time I tried to do it the functional way. You validate how well I did or not.
Based on your suggestions I now use an interface for the base definition:
and derive two types from it. The actual markup-element
```
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class MarkupElement : IMarkupElement
{
private readonly List _content = new List();
public MarkupElement(string name, IEnumerable content)
{
Name = name.NonEmptyOrNull() ?? throw new ArgumentNullException(nameof(name));
Add(content ?? throw new ArgumentNullException(nameof(content)));
Attributes = new Dictionary(StringComparer.OrdinalIgnoreCase);
}
private string DebuggerDisplay => $"";
#region IMarkupElement
public static IMarkupElement Builder => new NullMarkupElement();
public string Name { get; }
public IDictionary Attributes { get; }
public IMarkupElement Parent { get; set; }
public int Depth
{
get
{
var parent = Parent;
var depth = parent != null ? 1 : 0;
while ((parent = parent?.Parent) != null) depth++;
return depth;
}
}
#endregion
#region ICollection
public int Count => _content.Count;
public bool IsReadOnly => false;
public void Add(object item)
{
if (item == null) return;
switch (item)
{
case IMarkupElement e: e.Parent = this; break;
case
This time I tried to do it the functional way. You validate how well I did or not.
Based on your suggestions I now use an interface for the base definition:
public interface IMarkupElement : ICollection
{
string Name { get; }
IDictionary Attributes { get; }
IMarkupElement Parent { get; set; }
int Depth { get; }
}and derive two types from it. The actual markup-element
```
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class MarkupElement : IMarkupElement
{
private readonly List _content = new List();
public MarkupElement(string name, IEnumerable content)
{
Name = name.NonEmptyOrNull() ?? throw new ArgumentNullException(nameof(name));
Add(content ?? throw new ArgumentNullException(nameof(content)));
Attributes = new Dictionary(StringComparer.OrdinalIgnoreCase);
}
private string DebuggerDisplay => $"";
#region IMarkupElement
public static IMarkupElement Builder => new NullMarkupElement();
public string Name { get; }
public IDictionary Attributes { get; }
public IMarkupElement Parent { get; set; }
public int Depth
{
get
{
var parent = Parent;
var depth = parent != null ? 1 : 0;
while ((parent = parent?.Parent) != null) depth++;
return depth;
}
}
#endregion
#region ICollection
public int Count => _content.Count;
public bool IsReadOnly => false;
public void Add(object item)
{
if (item == null) return;
switch (item)
{
case IMarkupElement e: e.Parent = this; break;
case
Solution
NonEmptyOrNull()the name is ambiguous. Is it
!empty || null or is it !(empty or null)? Easy to get it wrong. I would just use regular !String.IsNullOrEmpty() - no ambiguity here.content ?? throw new ArgumentNullException(nameof(content))why content can not be null? I would assume that at some point you are going to add an element with no children/content. Should I pass
new IMarkupElement[0] in that case? I would rather just pass null (or pass nothing at all).public int Depth
{
get
{
var parent = Parent;
var depth = parent != null ? 1 : 0;
while ((parent = parent?.Parent) != null) depth++;
return depth;
}
}Can't you just return
Parent == null ? 0 : Parent.Depth + 1?Add(object item)IMHO, this method's implemntation has big wtfsurprise factor. Imagine if
ArrayList were to silently drop some of the items you've added based on some undocumented criteria. I suggest you:- Use
Add(object)only to add single object to the collection, because that's the contract defined byICollection. Use differentAddRangemethod to add multiple items.
throwif invalid item is added. Otherwise add the item to collection.
- Document any weird or unexpected behavior. You might have a reason to implement
Add(object)that way, but this reason is not obvious.
NullMarkupElement should not have public constructor since it is only used in specific place with specific purpose. You can also consider just straight up using ((IMarkupElement)null) as your "builder".Overall I think this implementation would be easier to understand/maintain if you were to "sanitize" during html construction, and not during rendering. Instead of having special rules on how to render non-
IMarkupElement objects, you should have special rules on how to convert those object to IMarkupElement. This way you should be able to work with strongly typed IMarkupElement : ICollection instead of IMarkupElement : ICollection.Code Snippets
NonEmptyOrNull()content ?? throw new ArgumentNullException(nameof(content))public int Depth
{
get
{
var parent = Parent;
var depth = parent != null ? 1 : 0;
while ((parent = parent?.Parent) != null) depth++;
return depth;
}
}Add(object item)Context
StackExchange Code Review Q#159043, answer score: 3
Revisions (0)
No revisions yet.