6
\$\begingroup\$

I have a list of labels for printing. Now I have go over the list and print the labels. The layout is specified by how many labels per page and per row on a page I can print.

@{
    var labels = new List<int> { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 };
    var labelsPerPage = 10;
    var labelsPerRow = 2;
    var labelCount = labels.Count;
}

@for (var i = 0; i < labelCount; i++) {
    if (i % labelsPerPage == 0) {
        @Html.Raw("<table class=\"page\">")
    }

    if (i % labelsPerRow == 0) {
        @Html.Raw("<tr>")
    }

    <td class="label">@PrintLabel(labels[i])</td>

    if (i % labelsPerRow == (labelsPerRow - 1)) {
        @Html.Raw("</tr>")
    }

    if (i % labelsPerPage == (labelsPerPage - 1)) {
        @Html.Raw("</table>")
    }
}

@helper PrintLabel(int label) {
    @label
}

Is there a better method (eg. using LINQ or nested for/foreach loops) for going through the list and doing something at the start and end of each page and each row?

I would prefer a nested structure to a flat one so it can be used inside a Razor template. The flat version fails here because Razor requires HTML tags to be closed at the end of a code block.

I'm not looking for performance here, I'd rather have more readable code and possibly getting rid of any Html.Raw() calls.

\$\endgroup\$
13
  • 1
    \$\begingroup\$ I like your fore loop the best, it's the cleanest code and you can control your page brakes based the way you are doing it now. Sure you can page in Linq but why, your code is faster, cleaner, will not new-up anonymous classes. I like the for loop, I'd even use it if I'd have to format it in html as your foreach can't make guesses of current index and ho many are left to come nor the size of the slip, or width of slip. You see the for loop a lot when having to make ATM or cash receipts \$\endgroup\$ Sep 3, 2019 at 15:54
  • 2
    \$\begingroup\$ I don't think it will re-open after that edit. Could you include an example of how you would use this? Is this current trivial example an actual use case? Also, it would be better to visualise the layout if you include the Console output. \$\endgroup\$
    – dfhwze
    Sep 3, 2019 at 17:38
  • 1
    \$\begingroup\$ Just for my understanding (if I get the idea to ask another question here at CR): Is it actually relevant for understanding the question that I output HTML tags (instead of printing eg. ZPL printing language commands) even if the code structure is the same? Yes this is real code already in production (except that the label contains more than just a number). What context was missing? The HTML? That it is actually used in a Razor template? I thought a C# code snippet would be easier to run than some Razor template code. \$\endgroup\$
    – Dehalion
    Sep 4, 2019 at 13:59
  • 2
    \$\begingroup\$ If you want to get most out of Code Review and the best input then it's in your interest to post the code that is as real as possible. One can do plenty of mistakes or not know many great APIs that you would never hear about if you hide the fact that you are working with html or razor pages. The more you tell us the better feedback you get. There is no such thing as irrelevant code here on Code Review. \$\endgroup\$
    – t3chb0t
    Sep 4, 2019 at 14:16
  • 2
    \$\begingroup\$ Nope, your problem is not not splitting a list into thee dimensions but to render a table. What you are describing is a typical xy problem meta.stackexchange.com/questions/66377/what-is-the-xy-problem you think the solution is a more efficient way to split a list but what you really want is a table and that splitting is only what you can think of right now so you believe you need to improve that. \$\endgroup\$
    – t3chb0t
    Sep 4, 2019 at 14:30

1 Answer 1

1
\$\begingroup\$

You're closing tags only when a row or table is full. Is data always going to fit the table size?

if (i % labelsPerRow == (labelsPerRow - 1)) {
    @Html.Raw("</tr>")
}

if (i % labelsPerPage == (labelsPerPage - 1)) {
    @Html.Raw("</table>")
}

You could make a functional helper and use that instead. Notice that after the loop, the row and table are ensured to be closed. The purpose is to allow the consumer to pick how to build a TAggregate from TSource items and observer methods. We're observing every item, table start/end and row start/end.

public static class GridRenderer
{
    public static TAggregate Render<TSource, TAggregate>(IEnumerable<TSource> source, TAggregate seed, int pageSize, int rowSize,
        Func<TSource, TAggregate, TAggregate> itemObserver,
        Func<TAggregate, TAggregate> beginPageObserver,
        Func<TAggregate, TAggregate> endPageObserver,
        Func<TAggregate, TAggregate> beginRowObserver,
        Func<TAggregate, TAggregate> endRowObserver)
    {
        if (source == null) throw new ArgumentNullException(nameof(source));
        if (pageSize <= 0) throw new ArgumentOutOfRangeException(nameof(pageSize));
        if (rowSize > pageSize) throw new ArgumentOutOfRangeException(nameof(rowSize));
        // .. check observers for null

        var items = source.ToList();
        var result = seed;

        if (!items.Any()) return result;

        for (var i = 0; i < items.Count; i++)
        {
            if (i % pageSize == 0)
            {
                result = beginPageObserver(result);
            }

            if (i % rowSize == 0)
            {
                result = beginRowObserver(result);
            }

            result = itemObserver(items[i], result);

            if ((i + 1) % rowSize == 0)
            {
                result = endRowObserver(result);
            }

            if ((i + 1) % pageSize == 0)
            {
                result = endPageObserver(result);
            }
        }

        if (items.Count % rowSize != 0)
        {
            result = endRowObserver(result);
        }

        if (items.Count % pageSize != 0)
        {
            result = endPageObserver(result);
        }

        return result;
    }
}

Let's say your data does not fit the table size and row size.

var renderer = new StringBuilder();
GridRenderer.Render(new[] {1, 2, 3, 4, 5}, renderer, 4, 2,
    (item, cur) => cur.AppendLine($"    <td class=\"label\">{item}</td>"),
    cur => cur.AppendLine("<table class=\"page\">"),
    cur => cur.AppendLine("</table>"),
    cur => cur.AppendLine("  <tr>"),
    cur => cur.AppendLine("  </tr>"));
var layout = renderer.ToString();

The grid still gets created correctly.

<table class="page">
  <tr>
    <td class="label">1</td>
    <td class="label">2</td>
  </tr>
  <tr>
    <td class="label">3</td>
    <td class="label">4</td>
  </tr>
</table>
<table class="page">
  <tr>
    <td class="label">5</td>
  </tr>
</table>

Note I've used a StringBuilder, but you could also use any other class to render the output. You could also change the flow a bit if you want an empty table when no data is available.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Your solution is great (generic, flexible) and thanks for pointing out unclosed rows/tables. I'd really like a nested version if possible so I can leverage Razor, but if there is none or nobody comes up with one I'll mark this one as an answer. \$\endgroup\$
    – Dehalion
    Sep 13, 2019 at 13:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.