-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: Use <( )> to embed code blocks in XML literals, string literals, and in any expression #422
Comments
For your specific example, I think support for ranges #418, #25, would have more general usefulness (e.g. apart from just XML literals). Something like: Dim x = <div>
<%= From i In (1 to 10) Select <p>@i</p>
%>
</div> I've done quite a bit of work with XML literals in my time; I have a couple of thoughts here about things I've learnt through years of having to maintain them:
|
@pricerc: <div class="container">
<%= (Function()
If Me.CatalogModel.CatalogItems.Any() Then
Return <zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<%= (Iterator Function()
For i = 0 To CatalogModel.CatalogItems.Count - 1
Yield _
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{i}]" %>/>
</div>
Next
End Function)() %>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
Else
Return _
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
End If
End Function)() %>
</div> It will be better if it becomes" <div class="container">
<(If Me.CatalogModel.CatalogItems.Any() Then
<zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<(For i = 0 To CatalogModel.CatalogItems.Count - 1
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{i}]" %>/>
</div>
Next)>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
Else
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
End If)>
</div> |
If I replace your <div Class="container">
<zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo" />
<div class="esh-catalog-items row">
<div class="esh-catalog-item col-md-4">
<partial name="_product" for="CatalogItems[0]" />
</div>
<div class="esh-catalog-item col-md-4">
<partial name="_product" for="CatalogItems[1]" />
</div>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo" />
</zml>
</div>
--or, for an empty list:
<div Class="container">
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
</div> I've loaded your code, and two alternatives into LinqPad. Here they are: Sub Main
Original.Dump()
Alternate.Dump()
GetZml.Dump()
End Sub
Dim CatalogItems As New List(Of String) From {} ' "Foo", "Bar"}
Dim Original As XElement =
<div class="container">
<%= (Function()
If CatalogItems.Any() Then
Return <zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<%= (Iterator Function()
For i = 0 To CatalogItems.Count - 1
Yield _
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{i}]" %>/>
</div>
Next
End Function)() %>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
Else
Return _
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
End If
End Function)() %>
</div>
Dim Alternate As XElement =
If(CatalogItems.Any(),
<div Class="container">
<zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<%=
From Item In CatalogItems Select
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{CatalogItems.IndexOf(Item)}]" %>/>
</div>
%>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
</div> , 'else
<div Class="container">
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
</div>
)
Function GetZML() As XElement
If CatalogItems.Any() Then Return _
<div Class="container">
<zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<%=
From Item In CatalogItems Select
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{CatalogItems.IndexOf(Item)}]" %>/>
</div>
%>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
</div>
Return _
<div Class="container">
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
</div>
End Function
For me, I think the method version is the easiest to read, because it has space between the two conditions, and makes it easier to process them separately. But both alternatives a) move the conditional to the outside, and b) use query expressions instead of anonymous methods to produce a more legible (for me) bit of code than your example. They both also require less code. Getting a bit off-topic: I don't like the |
@pricerc
I would use ZML loops in such cases, but in this version of the example, I am showing how to do it with vb. |
I beg to differ. The number of layers of loop makes no difference, except that the more layers deep you go, the more important it is to break down the problem into smaller pieces to make them manageable. The techniques I've used here, I've used many times before to achieve the result you're after - XML with a specific structure, almost always more complicated than your quite trivial example. Usually, in my case it's something like sales orders being sent to a web service, but the concepts are the same - constructing well-formed XML with multiple layers, and loops at multiple layers. Multiple nested loops in the same method are usually (in my experience) a sign of code that needs refactoring - a single method should do as little as possible, ideally just one thing. If you have, for example, a sales order, with say, some header information and then some order lines, and the order lines might have components, and the components might have serial numbers, I would have:
It means I can modify and (probably more importantly) test each one individually without interfering with the other parts. In general, I make it my goal to never have a method that exceeds one screen in height. I frequently fail (e.g. because I have to build XML for a sales order header that has 50 required elements), but that doesn't change the goal, because it makes for easier maintenance in the long term. |
@pricerc |
Yes, because that's what you're creating - output in the form of XML, I think I possibly don't understand your point with that statement.
Input is what the users of your application type into their computers. What we're talking about is the program that the user interfaces with. The most important thing is that the program's source code needs to be readable and maintainable. The XML literals in your source file are not 'input', they are syntax sugar for LINQ to XML objects.
I disagree. But that's ok. What I described is exactly what I have done for a specific project. It was not overkill, it was the right amount of effort to create an easily-maintained, structured program that produced the right XML, reliably. If you try to produce a >100 line XML document all in one big VB method, you're in for a world of hurt with a maintenance nightmare. To think that the point of XML literals is to allow you to code a whole XML document into your VB source file is a very limiting view. The true power comes from being able to build the various pieces of a large document in small manageable chunks, using XML literals instead of having to use XML classes (System.Xml or System.Xml.Linq) explicitly. |
@pricerc |
@VBAndCs , of course we use XML literals in different ways, and that's awesome. But do you really think that this: If(CatalogItems.Any(),
<div Class="container">
<zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<%= From i In (1 To CatalogItems.Count - 1) Select
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{i}]" %>/>
</div>
%>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
</div> , 'else
<div Class="container">
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
</div>
) is more complicated and harder to read than this?: <div class="container">
<(If CatalogItems.Any() Then
<zml>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
<div class="esh-catalog-items row">
<(For i = 0 To CatalogItems.Count - 1
<div class="esh-catalog-item col-md-4">
<partial name="_product" for=<%= $"CatalogItems[{i}]" %>/>
</div>
Next)>
</div>
<partial name="_pagination" for="CatalogModel.PaginationInfo"/>
</zml>
Else
<div Class="esh-catalog-items row">
THERE ARE NO RESULTS THAT MATCH YOUR SEARCH
</div>
End If)>
</div> Both would produce the same output, within a single statement. BTW, I also think that the improvement you're proposing could be addressed with block expressions(#109), which even has some examples using XML literals. |
Ok. Let's try another example. How would you represent this: Function GetVbXml() As XElement
Return _
_
<zml xmlns:z="zml">
<z:model type="ExternalLoginsViewModel"/>
<z:title>Manage your external logins</z:title>
<partial name="_StatusMessage" for="StatusMessage"/>
<%= (Function()
If CurrentLogins?.Count > 0 Then
Return <zml xmlns:z="zml">
<h4>Registered Logins</h4>
<table class="table">
<tbody>
<%= (Iterator Function()
For Each login In CurrentLogins
Yield <tr>
<td>@login.LoginProvider</td>
<td>
<%= (Function()
If ShowRemoveButton Then
Return <form asp-action="RemoveLogin" method="post">
<div>
<input asp-for="@login.LoginProvider" name="LoginProvider" type="hidden"/>
<input asp-for="@login.ProviderKey" name="ProviderKey" type="hidden"/>
<button type="submit" class="btn btn-default" title="Remove this @login.LoginProvider login from your account">Remove</button>
</div>
</form>
Else
Return " "
End If
End Function)() %>
</td>
</tr>
Next
End Function)() %>
</tbody>
</table>
</zml>
End If
If OtherLogins?.Count > 0 Then
Return <zml>
<h4>Add another service to log in.</h4>
<hr/>
<form asp-action="LinkLogin" method="post" class="form-horizontal">
<div id="socialLoginList">
<p>
<%= (Iterator Function()
For Each provider In OtherLogins
Yield <button type="submit" class="btn btn-default" name="provider" value="@provider.Name" title="Log in using your @provider.DisplayName account">@provider.DisplayName</button>
Next
End Function)() %>
</p>
</div>
</form>
</zml>
End If
Return Nothing
End Function)() %>
</zml>
End Function Of course I want it to be: Function GetVbXml() As XElement
Return _
_
<zml xmlns:z="zml">
<z:model type="ExternalLoginsViewModel"/>
<z:title>Manage your external logins</z:title>
<partial name="_StatusMessage" for="StatusMessage"/>
<(If CurrentLogins?.Count > 0 Then
<zml xmlns:z="zml">
<h4>Registered Logins</h4>
<table class="table">
<tbody>
<(For Each login In CurrentLogins
<tr>
<td>@login.LoginProvider</td>
<td>
<(If ShowRemoveButton Then
<form asp-action="RemoveLogin" method="post">
<div>
<input asp-for="@login.LoginProvider" name="LoginProvider" type="hidden"/>
<input asp-for="@login.ProviderKey" name="ProviderKey" type="hidden"/>
<button type="submit" class="btn btn-default" title="Remove this @login.LoginProvider login from your account">Remove</button>
</div>
</form>
Else
" "
End If)>
</td>
</tr>
Next)>
</tbody>
</table>
</zml>
End If)>
If OtherLogins?.Count > 0 Then
<zml>
<h4>Add another service to log in.</h4>
<hr/>
<form asp-action="LinkLogin" method="post" class="form-horizontal">
<div id="socialLoginList">
<p>
<(For Each provider In OtherLogins
<button type="submit" class="btn btn-default" name="provider" value="@provider.Name" title="Log in using your @provider.DisplayName account">@provider.DisplayName</button>
Next)>
</div>
</form>
</zml>
End If)>
</zml>
End Function |
@VBAndCs, you didn't actually answer my question. Nonetheless, I'm prepared to take up your challenge: if you can provide a complete working example of that code, that I can run in LinqPad (I need to be able to paste the code into LinqPad, press 'Execute' and have it call your |
I've just spotted something. Is this bit that I wrote the reason why you thought I was focussed on the output?:
|
I suggest to embed VB code blocks in xml literals like this
I think the
<( )>
is a suitable quote. The <%= %> will continue used to embed values. The<( )>
will grantee not to break any existing code. It is supposed to return a value, so it is lowered to a function call:Notes:
<( )>
, implicit return is assumed.Return Nothing
is assumed. This will allow writing Subs also to define some variables in the XML literal scope.@
rules to switch between text, VB Code, and XML Literals. Again, since the<( )>
is new quote, there is no fear to break any existing code.Note that This proposal can be implemented using @AnthonyDGreen 's Top-Level Code Prototype can achieve this proposal:
and many more, like embedding code in interpolated strings
The text was updated successfully, but these errors were encountered: