Skip to main content
specialized in white space and blank lines (python or C#) ;-)
Source Link
dfhwze
  • 13.9k
  • 3
  • 38
  • 101

I find you should keep the library code in the model and only parts that are relevant to views inside them.


In your case there is actually a third option. Let OnGet only handle the file. Remove OrderBy as this is a view's matter how it's displayed. Additionally initialize itemListReportages to an empty list if a file was empty. It'll save you the trouble of null checks later.

public void OnGet()
{
    PageSettings = (PagesSettings)HttpContext.Items["pagesettings"];

    if (PageSettings == null)
    {
        return;
    }

        
    var fileContent = _fileService.GetFileContentCached(PageSettings.DataFile);

    itemListReportages = 
        fileContent?.Length > 0
            ? JsonConvert.DeserializeObject<List<Reportage>>(fileContent).Where(l => l.Published).ToList()
            : new List<Reportage>();
            
}

We don't see whether you initialize itemListReportages but this view could crash because you don't use the null propagation operator here: ?.Count

@for (var i = 0; i < Model.itemListReportages.Count; i++)
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@Model.itemListReportages[i].BkgImage');"></div>
        <div class="itemtxt">
            <span>@Model.itemListReportages[i].Text</span>
        </div>
    </div>
}

With a non-null list you can turn this into a foreach loop and put OrderBy here. It'll make the code shorter too as you now have an item variable and no longer have to acces them by index like Model.itemListReportages[i]:

@foreach (var item in Model.itemListReportages.OrderBy(x => x.Name))
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@item.BkgImage');"></div>
        <div class="itemtxt">
            <span>@item.Text</span>
        </div>
    </div>
}

I find you should keep the library code in the model and only parts that are relevant to views inside them.


In your case there is actually a third option. Let OnGet only handle the file. Remove OrderBy as this is a view's matter how it's displayed. Additionally initialize itemListReportages to an empty list if a file was empty. It'll save you the trouble of null checks later.

public void OnGet()
{
    PageSettings = (PagesSettings)HttpContext.Items["pagesettings"];

    if (PageSettings == null)
    {
        return;
    }

        
    var fileContent = _fileService.GetFileContentCached(PageSettings.DataFile);

    itemListReportages = 
        fileContent?.Length > 0
            ? JsonConvert.DeserializeObject<List<Reportage>>(fileContent).Where(l => l.Published).ToList()
            : new List<Reportage>();
            
}

We don't see whether you initialize itemListReportages but this view could crash because you don't use the null propagation operator here: ?.Count

@for (var i = 0; i < Model.itemListReportages.Count; i++)
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@Model.itemListReportages[i].BkgImage');"></div>
        <div class="itemtxt">
            <span>@Model.itemListReportages[i].Text</span>
        </div>
    </div>
}

With a non-null list you can turn this into a foreach loop and put OrderBy here. It'll make the code shorter too as you now have an item variable and no longer have to acces them by index like Model.itemListReportages[i]:

@foreach (var item in Model.itemListReportages.OrderBy(x => x.Name))
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@item.BkgImage');"></div>
        <div class="itemtxt">
            <span>@item.Text</span>
        </div>
    </div>
}

I find you should keep the library code in the model and only parts that are relevant to views inside them.


In your case there is actually a third option. Let OnGet only handle the file. Remove OrderBy as this is a view's matter how it's displayed. Additionally initialize itemListReportages to an empty list if a file was empty. It'll save you the trouble of null checks later.

public void OnGet()
{
    PageSettings = (PagesSettings)HttpContext.Items["pagesettings"];

    if (PageSettings == null)
    {
        return;
    }

    var fileContent = _fileService.GetFileContentCached(PageSettings.DataFile);

    itemListReportages = 
        fileContent?.Length > 0
            ? JsonConvert.DeserializeObject<List<Reportage>>(fileContent).Where(l => l.Published).ToList()
            : new List<Reportage>();
}

We don't see whether you initialize itemListReportages but this view could crash because you don't use the null propagation operator here: ?.Count

@for (var i = 0; i < Model.itemListReportages.Count; i++)
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@Model.itemListReportages[i].BkgImage');"></div>
        <div class="itemtxt">
            <span>@Model.itemListReportages[i].Text</span>
        </div>
    </div>
}

With a non-null list you can turn this into a foreach loop and put OrderBy here. It'll make the code shorter too as you now have an item variable and no longer have to acces them by index like Model.itemListReportages[i]:

@foreach (var item in Model.itemListReportages.OrderBy(x => x.Name))
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@item.BkgImage');"></div>
        <div class="itemtxt">
            <span>@item.Text</span>
        </div>
    </div>
}
Source Link
t3chb0t
  • 44.3k
  • 9
  • 84
  • 190

I find you should keep the library code in the model and only parts that are relevant to views inside them.


In your case there is actually a third option. Let OnGet only handle the file. Remove OrderBy as this is a view's matter how it's displayed. Additionally initialize itemListReportages to an empty list if a file was empty. It'll save you the trouble of null checks later.

public void OnGet()
{
    PageSettings = (PagesSettings)HttpContext.Items["pagesettings"];

    if (PageSettings == null)
    {
        return;
    }

        
    var fileContent = _fileService.GetFileContentCached(PageSettings.DataFile);

    itemListReportages = 
        fileContent?.Length > 0
            ? JsonConvert.DeserializeObject<List<Reportage>>(fileContent).Where(l => l.Published).ToList()
            : new List<Reportage>();
            
}

We don't see whether you initialize itemListReportages but this view could crash because you don't use the null propagation operator here: ?.Count

@for (var i = 0; i < Model.itemListReportages.Count; i++)
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@Model.itemListReportages[i].BkgImage');"></div>
        <div class="itemtxt">
            <span>@Model.itemListReportages[i].Text</span>
        </div>
    </div>
}

With a non-null list you can turn this into a foreach loop and put OrderBy here. It'll make the code shorter too as you now have an item variable and no longer have to acces them by index like Model.itemListReportages[i]:

@foreach (var item in Model.itemListReportages.OrderBy(x => x.Name))
{
    <div class="item">
        <div class="itembkg" style="background-image: url('@item.BkgImage');"></div>
        <div class="itemtxt">
            <span>@item.Text</span>
        </div>
    </div>
}