patterncsharpMinor
Displaying separated multi-line text fields
Viewed 0 times
multilinefieldstextdisplayingseparated
Problem
The following code grabs the HTML-encoded, multi-line content of a text field. It then splits the content on line breaks, and finally displays it with commas as separators.
I would like to have this code reviewed to see if there is a simpler way to grab the string, split and display it.
Note - I hated the idea of writing 2 collection iterations, but I found if a user in the CMS entered one line of real data, and then a couple of empty line breaks, it would throw the array length off and display a comma on a single item e.g. output should look like this :
I would like to have this code reviewed to see if there is a simpler way to grab the string, split and display it.
Note - I hated the idea of writing 2 collection iterations, but I found if a user in the CMS entered one line of real data, and then a couple of empty line breaks, it would throw the array length off and display a comma on a single item e.g. output should look like this :
- Correct: Allowed Uses: example 1 | Allowed Uses: example 1, example 2
- Incorrect: Allowed Uses: example 1, | Allowed Uses: example 1, example 2,
var retailBreaks = umbraco.library.ReplaceLineBreaks(Model.landAllowedUses);
string[] retailBreaksSplit = retailBreaks.Split(new string[] { "
" }, StringSplitOptions.RemoveEmptyEntries);
int breaksLength = 0;
List repeatableProperty = new List();
foreach (string s in retailBreaksSplit)
{
if (!string.IsNullOrWhiteSpace(s))
{
repeatableProperty.Add(s);
}
}
breaksLength = repeatableProperty.Count();
Allowed Uses:
@foreach (string s in repeatableProperty)
{
@s.Trim()@((breaksLength > 1 && repeatableProperty.Last() != s) ? ", " : string.Empty)
}
Solution
You already have got a nice and simple solution from @GavinCoates so I only would like to add a couple of comments about your code for your future coding experience.
-
You should consequently use either full types for your variables or the
-
The same applies to the
-
You started using good variable names but didn't continue in your loops. This would look and read mutch better:
-
If you have collections or lists etc. you should name the variables using plural form to indicate it's more then one element. This would be a better name for the list:
-
The final thing I would change is the last loop. I would create a helper variable for the text instead of putting it inside the span. It's been long time since I last coded with
-
You should consequently use either full types for your variables or the
var keyword. In your code the var would greatly improve the readabilty as all types are obvious here and don't need to declared twice:var retailBreaks = umbraco.library.ReplaceLineBreaks(Model.landAllowedUses);
var retailBreaksSplit = retailBreaks.Split(new string[] { "" }, StringSplitOptions.RemoveEmptyEntries);
var breaksLength = 0;
var repeatableProperty = new List();-
The same applies to the
foreach-loops:foreach (var s in retailBreaksSplit)-
You started using good variable names but didn't continue in your loops. This would look and read mutch better:
foreach (var retailBreak in retailBreaksSplit)-
If you have collections or lists etc. you should name the variables using plural form to indicate it's more then one element. This would be a better name for the list:
var repeatableProperties = new List();-
The final thing I would change is the last loop. I would create a helper variable for the text instead of putting it inside the span. It's been long time since I last coded with
ASP.NET so please forgive me if I got the syntax wrong.@foreach (var repeatableProperty in repeatableProperties)
{
var text = @((breaksLength > 1 && repeatableProperty.Last() != s) ? ", " : string.Empty);
@s.Trim()@text
}Code Snippets
var retailBreaks = umbraco.library.ReplaceLineBreaks(Model.landAllowedUses);
var retailBreaksSplit = retailBreaks.Split(new string[] { "<br/>" }, StringSplitOptions.RemoveEmptyEntries);
var breaksLength = 0;
var repeatableProperty = new List<String>();foreach (var s in retailBreaksSplit)foreach (var retailBreak in retailBreaksSplit)var repeatableProperties = new List<String>();@foreach (var repeatableProperty in repeatableProperties)
{
var text = @((breaksLength > 1 && repeatableProperty.Last<string>() != s) ? ", " : string.Empty);
<span class="comma-list">@s.Trim()@text </span>
}Context
StackExchange Code Review Q#104932, answer score: 2
Revisions (0)
No revisions yet.