1
\$\begingroup\$

I have a complex nested object with an array of objects coming from an API. But the grid tool best works with flattened data. So I came up with this logic that will build the correct object for the grid tool. To achieve this, it heavily relies on foreach loops and adding object to list object. Please review the following code snippets. I would be happy for any kind of feedback since this is quite critical a part of my application.

Running app

using System;
using System.Linq;
using System.Collections.Generic;
                        
public class Program
{
    public static void Main()
    {
        Console.WriteLine("Processing Data");
        
        List<PanelsMeetingListsViewModel> ListOfPanelsMeeting = new List<PanelsMeetingListsViewModel>();
        List<PanelsMeetingViewModel> MeetingVM = new List<PanelsMeetingViewModel>();
        MeetingVM.Add(new PanelsMeetingViewModel() { PanelName = "A1", MeetingDate = "Mon" });
        MeetingVM.Add(new PanelsMeetingViewModel() { PanelName = "A2", MeetingDate = "Tues" });
        MeetingVM.Add(new PanelsMeetingViewModel() { PanelName = "A2", MeetingDate = "Wed" });
        ListOfPanelsMeeting.Add(new PanelsMeetingListsViewModel() { Name = "A", TypeId = 1, PanelsMeetingViewModel = MeetingVM});
        
        List<ParentModel> result = new List<ParentModel>();
        foreach (var PanelType in ListOfPanelsMeeting)
        {
            ParentModel PanelsMeetingParent = new ParentModel();
            
            ParentModel ParentGrid = new ParentModel();
            PanelsMeetingParent.TitleName = PanelType.Name;
            
            List<GridDetailsModel> GridDetailsListModel = new List<GridDetailsModel>();
            
            GridDetailsModel PanelMeetingGridDetails = new GridDetailsModel();

            PanelMeetingGridDetails.GridName = PanelType.TypeId.ToString();
            PanelMeetingGridDetails.GridID = PanelType.TypeId;
        
            List<DetailsModel> ListOfGridDetails = new List<DetailsModel>();
            foreach (var PanelMeeting in PanelType.PanelsMeetingViewModel)
            {
                 DetailsModel Details = new DetailsModel();
                 Details.DetailsName = PanelMeeting.PanelName;
                 Details.DetailsType = PanelMeeting.Type ==null ? "": PanelMeeting.Type;
                 ListOfGridDetails.Add(Details);
            }
            
            PanelMeetingGridDetails.Details = ListOfGridDetails;
            GridDetailsListModel.Add(PanelMeetingGridDetails);

            PanelsMeetingParent.GridData = GridDetailsListModel;
            
            result.Add(PanelsMeetingParent);
        }
        
        foreach (var ParentModel in result)
        {
            Console.WriteLine(ParentModel.TitleName);
            foreach (var GridDetailsModel in ParentModel.GridData)
            {
                Console.WriteLine(GridDetailsModel.GridName);
                foreach (var DetailsModel in GridDetailsModel.Details)
                {
                    Console.WriteLine(DetailsModel.DetailsName);
                }
            }  
        }
    }
}
public class PanelsMeetingListsViewModel
{
    public string Name { get; set; }
    public int TypeId { get; set; }
    public List<PanelsMeetingViewModel> PanelsMeetingViewModel { get; set; }
}

public class PanelsMeetingViewModel
{
    public string PanelName { get; set; }
    public string MeetingDate { get; set; }
    public string Type { get; set; }
}

public class ParentModel
{
    public string TitleName { get; set; }
    public List<GridDetailsModel> GridData { get; set; }
}
        
public class GridDetailsModel
{
    public int GridID { get; set; }
    public string GridName { get; set; }
    public List<DetailsModel> Details { get; set; }
}
        
public class DetailsModel
{
    public int DetailsID { get; set; }
    public string DetailsName { get; set; }
    public string DetailsType { get; set; }
}
\$\endgroup\$
6
  • \$\begingroup\$ Your code is broken, it's using 'DetailsModel' in the initialization list for GridDetailsModel. GridDetailsModel doesn't have a property of DetailsModel, and the Details property is of a different type. Additionally, your foreach loop isn't really doing anything since all it's doing is creating locally scoped variables and ending. I'm not sure what you're trying to do here? \$\endgroup\$
    – AotN
    Dec 5, 2020 at 19:35
  • \$\begingroup\$ Just to let you know, it would be much easier if you used AutoMapper instead! you could use it to map your models or flatten complex objects with ease!. \$\endgroup\$
    – iSR5
    Dec 5, 2020 at 22:13
  • \$\begingroup\$ Result object is return to the view for the grid control \$\endgroup\$
    – Jefferson
    Dec 6, 2020 at 0:26
  • \$\begingroup\$ I dont have AutoMapper install is there anything else I can do? \$\endgroup\$
    – Jefferson
    Dec 6, 2020 at 0:27
  • \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Dec 8, 2020 at 9:33

1 Answer 1

2
\$\begingroup\$

A few things that have caught my attention:

  1. You write:

    But the grid tool best works with flattened data

    Since you are turning a two-level hierarchy (PanelsMeetingListsViewModel 1 -- * PanelsMeetingViewModel ) into a three-level hierarchy (ParentModel 1 -- * GridDetailsModel 1 -- * DetailsModel) I fail to see where the flattening is supposedly happening. If anything you nest things even deeper.

    Also the code only ever adds one GridDetailsModel to a ParentModel so it's not clear why there is a list of GridDetailsModel

  2. Expressions of the form x = A == null ? B : A can be written more concisely using the null-coalescing operator as x = A ?? B

  3. I would de-compose the big loop into smaller chunks that are each responsible for turning one specific type into another. If we start with the innermost objects we can introduce an extension method that transforms a PanelsMeetingViewModel into a DetailsModel:

    public static DetailsModel ToDetailsModel(this PanelsMeetingViewModel meetingVM)
    {
        return new DetailsModel
        {
            DetailsName = meetingVM.PanelName,
            DetailsType = meetingVM.Type ?? ""
        };
    }
    

    Since the intermediate GridDetailsModel has no correspondent match in the original hierarchy we move one level up to translate a PanelsMeetingListsViewModel into a ParentModel:

    public static ParentModel ToParentModel(this PanelsMeetingListsViewModel listVM)
    {
        var res = new ParentModel() { TitleName = listVM.Name };
    
        var gridData = new GridDetailsModel()
                        {
                            GridID = listVM.TypeId, 
                            GridName = listVM.TypeId.ToString(),
                            Details = listVM.PanelsMeetingViewModel
                                            .Select(x => x.ToDetailsModel())
                                            .ToList()
                        };
        res.GridData = new List<GridDetailsModel> { gridData };
    
        return res;
    }
    

    These two extension methods now allow us to write the main loop simply as:

    var result = ListOfPanelsMeeting.Select(x => x.ToParentModel()).ToList();
    

    I've chosen extension methods so that the actual objects themselves don't need to have the knowledge how to transform themselves into other objects yet they allow us to elegantly write the transformation as a nice-to-read LINQ statement.

    Since the methods themselves are smaller only effectively only do one thing they are arguably easier to read and follow. It took me quite some time to dissect the big for-loop which wasn't made any easier by using very verbose type names that all start with the same prefix.

Refactored code can be found here: https://dotnetfiddle.net/Kgyfue

\$\endgroup\$
3
  • \$\begingroup\$ is that the correct Refactored code link? \$\endgroup\$
    – Jefferson
    Dec 8, 2020 at 16:23
  • \$\begingroup\$ @Jefferson - erm well it was meant to be but clearly I don't know how to properly use dotnetfiddle, will correct this shortly \$\endgroup\$
    – ChrisWue
    Dec 8, 2020 at 18:23
  • \$\begingroup\$ @Jefferson fiddle code fixed \$\endgroup\$
    – ChrisWue
    Dec 10, 2020 at 21:57

Your Answer

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

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