Can this method be refactored to use a lamba?

Question

I have this C# function:

private void BuildHistoryWeek_Students(CLMExplorerHistory record, ref MSAHistoryItem historyItem)
{
    try
    {
        bool bSampleConversationVideo = false;
        if (record.StudentItem1Description == _trans.IC_Video ||
            record.StudentItem1Description == _trans.RV_Video)
        {
            bSampleConversationVideo = true;
        }

        // AYFM - Class 1
        if (historyItem.Classes >= 1)
        {
            historyItem.StudentItems1.Add(
                new MSAHistoryItemStudent
                {
                    Name = record.BibleReadingClass1Name,
                    Study = record.BibleReadingStudy,
                    Type = _transMSA.BibleReading
                });

            if (record.StudentItem1Description != _trans.Video)
            {
                historyItem.StudentItems1.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem1Class1StudentName,
                        Assistant = record.StudentItem1Class1AssistantName,
                        Study = record.StudentItem1Study,
                        Type = record.StudentItem1Description,
                        SampleConversationVideo = bSampleConversationVideo
                    });
            }

            if (record.StudentItem2Description != string.Empty)
            {
                historyItem.StudentItems1.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem2Class1StudentName,
                        Assistant = record.StudentItem2Class1AssistantName,
                        Study = record.StudentItem2Study,
                        Type = record.StudentItem2Description
                    });
            }

            if (record.StudentItem3Description != string.Empty)
            {
                historyItem.StudentItems1.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem3Class1StudentName,
                        Assistant = record.StudentItem3Class1AssistantName,
                        Study = record.StudentItem3Study,
                        Type = record.StudentItem3Description
                    });
            }

            if (record.StudentItem4Description != string.Empty)
            {
                historyItem.StudentItems1.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem4Class1StudentName,
                        Assistant = record.StudentItem4Class1AssistantName,
                        Study = record.StudentItem4Study,
                        Type = record.StudentItem4Description
                    });
            }
        }


        // AYFM - Class 2
        if (historyItem.Classes >= 2)
        {
            historyItem.StudentItems2.Add(
                new MSAHistoryItemStudent
                {
                    Name = record.BibleReadingClass2Name,
                    Study = record.BibleReadingStudy,
                    Type = _transMSA.BibleReading
                });

            if (record.StudentItem1Description != _trans.Video)
            {
                historyItem.StudentItems2.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem1Class2StudentName,
                        Assistant = record.StudentItem1Class2AssistantName,
                        Study = record.StudentItem1Study,
                        Type = record.StudentItem1Description,
                        SampleConversationVideo = bSampleConversationVideo
                    });
            }

            if (record.StudentItem2Description != string.Empty)
            {
                historyItem.StudentItems2.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem2Class2StudentName,
                        Assistant = record.StudentItem2Class2AssistantName,
                        Study = record.StudentItem2Study,
                        Type = record.StudentItem2Description
                    });
            }

            if (record.StudentItem3Description != string.Empty)
            {
                historyItem.StudentItems2.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem3Class2StudentName,
                        Assistant = record.StudentItem3Class2AssistantName,
                        Study = record.StudentItem3Study,
                        Type = record.StudentItem3Description
                    });
            }

            if (record.StudentItem4Description != string.Empty)
            {
                historyItem.StudentItems2.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem4Class2StudentName,
                        Assistant = record.StudentItem4Class2AssistantName,
                        Study = record.StudentItem4Study,
                        Type = record.StudentItem4Description
                    });
            }
        }

        // AYFM - Class 3
        if (historyItem.Classes == 3)
        {
            historyItem.StudentItems3.Add(
                new MSAHistoryItemStudent
                {
                    Name = record.BibleReadingClass3Name,
                    Study = record.BibleReadingStudy,
                    Type = _transMSA.BibleReading
                });

            if (record.StudentItem1Description != _trans.Video)
            {
                historyItem.StudentItems3.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem1Class3StudentName,
                        Assistant = record.StudentItem1Class3AssistantName,
                        Study = record.StudentItem1Study,
                        Type = record.StudentItem1Description,
                        SampleConversationVideo = bSampleConversationVideo
                    });
            }

            if (record.StudentItem2Description != string.Empty)
            {
                historyItem.StudentItems3.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem2Class3StudentName,
                        Assistant = record.StudentItem2Class3AssistantName,
                        Study = record.StudentItem2Study,
                        Type = record.StudentItem2Description
                    });

            }

            if (record.StudentItem3Description != string.Empty)
            {
                historyItem.StudentItems3.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem3Class3StudentName,
                        Assistant = record.StudentItem3Class3AssistantName,
                        Study = record.StudentItem3Study,
                        Type = record.StudentItem3Description
                    });
            }

            if (record.StudentItem4Description != string.Empty)
            {
                historyItem.StudentItems3.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = record.StudentItem4Class3StudentName,
                        Assistant = record.StudentItem4Class3AssistantName,
                        Study = record.StudentItem4Study,
                        Type = record.StudentItem4Description
                    });
            }
        }
    }
    catch (System.Exception ex)
    {
        SimpleLog.Log(ex);
    }
}

It is reptative because it is doing the same thing for three "classes". But, each class is in a different variable:

historyItem.StudentItems1
historyItem.StudentItems2
historyItem.StudentItems3

And for each class, the set of record variables changes:

BibleReadingClass1Name
StudentItem1Class1StudentName
StudentItem1Class1AssistantName
StudentItem2Class1StudentName
StudentItem2Class1AssistantName
StudentItem3Class1StudentName
StudentItem3Class1AssistantName
StudentItem4Class1StudentName
StudentItem4Class1AssistantName

The other two classes name Class2 and Class3 respectively. I know I can create a method which is passed all of the properties as paramaters and call the method 3 times. But can I make changes within the method itself (like a lambda) to do this?

Are you aware of [local functions](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/local-functions)? — canton7, Jan 11 '22 at 09:20
Take a look at https://learn.microsoft.com/en-us/dotnet/api/system.func-2?view=net-6.0 and https://learn.microsoft.com/en-us/dotnet/api/system.action?view=net-6.0 — PawZaw, Jan 11 '22 at 10:01
The refactoring needs to be done in a different place. It's not your function that sucks, it's the definition of CLMExplorerHistory — Thomas Weller, Jan 11 '22 at 10:04
In your update you pass different type than declared in the function — PawZaw, Jan 11 '22 at 10:05
@ThomasWeller The `CLMExplorerHistory` is simply a CSV record set that I have to work with from another source. — Andrew Truckle, Jan 11 '22 at 10:16
@PawZaw I am still having an issue. I corrected the parameter to the local function itself but when I use the function it still says the same error. See updated question. — Andrew Truckle, Jan 11 '22 at 10:17
The internal data structure does not need to represent the structure of the CSV file in a 1:1 fashion. Just make sure you can read and write in a compatible way. — Thomas Weller, Jan 11 '22 at 10:25
I think that you're attempting to use the csv data structure directly rather than look ate possible areas of duplication that would indicate the obvious place for implementing a class type in order to allow for code reuse. Could you post up a snippet of example data for your CSV file to indicate your starting point? — ChrisBD, Jan 11 '22 at 11:07
@ChrisBD I have started a new question to improve the way the CSV file is read: https://stackoverflow.com/q/70684344/2287576 — Andrew Truckle, Jan 12 '22 at 15:39
@PawZaw I have simplified my question and added an answer. Can some of these comments be reduced? Thanks. — Andrew Truckle, Jan 12 '22 at 15:40
@ThomasWeller I have started a new question to improve the way the CSV file is read: stackoverflow.com/q/70684344/2287576 — Andrew Truckle, Jan 12 '22 at 15:41

score 0 · Accepted Answer · answered Jan 12 '22 at 15:19

In the comments it was suggested that I look into local functions.

I had a go and initially hit a issue with one of the parameters:

I have since learned from the answer to this question (Passing properties by reference in C#) why I could not pass the property by reference. Using the first suggestion there I have now come up with this working local function:

private void BuildHistoryWeek_Students(CLMExplorerHistory record, ref MSAHistoryItem historyItem)
{
    try
    {
        bool bSampleConversationVideo = false;
        if (record.StudentItem1Description == _trans.IC_Video ||
            record.StudentItem1Description == _trans.RV_Video)
        {
            bSampleConversationVideo = true;
        }
                       
        historyItem.StudentItems1 = BuildHistoryWeek_Students_Class(
            bSampleConversationVideo,
            record.BibleReadingClass1Name, record.BibleReadingStudy,
            record.StudentItem1Class1StudentName, record.StudentItem1Class1AssistantName, record.StudentItem1Study, record.StudentItem1Description,
            record.StudentItem2Class1StudentName, record.StudentItem2Class1AssistantName, record.StudentItem2Study, record.StudentItem2Description,
            record.StudentItem3Class1StudentName, record.StudentItem3Class1AssistantName, record.StudentItem3Study, record.StudentItem3Description,
            record.StudentItem4Class1StudentName, record.StudentItem4Class1AssistantName, record.StudentItem4Study, record.StudentItem4Description);

        if(historyItem.Classes >= 2)
        {
            historyItem.StudentItems2 = BuildHistoryWeek_Students_Class(
                bSampleConversationVideo,
                record.BibleReadingClass2Name, record.BibleReadingStudy,
                record.StudentItem1Class2StudentName, record.StudentItem1Class2AssistantName, record.StudentItem1Study, record.StudentItem1Description,
                record.StudentItem2Class2StudentName, record.StudentItem2Class2AssistantName, record.StudentItem2Study, record.StudentItem2Description,
                record.StudentItem3Class2StudentName, record.StudentItem3Class2AssistantName, record.StudentItem3Study, record.StudentItem3Description,
                record.StudentItem4Class2StudentName, record.StudentItem4Class2AssistantName, record.StudentItem4Study, record.StudentItem4Description);
        }

        if(historyItem.Classes == 3)
        {
            historyItem.StudentItems2 = BuildHistoryWeek_Students_Class(
                bSampleConversationVideo,
                record.BibleReadingClass2Name, record.BibleReadingStudy,
                record.StudentItem1Class3StudentName, record.StudentItem1Class3AssistantName, record.StudentItem1Study, record.StudentItem1Description,
                record.StudentItem2Class3StudentName, record.StudentItem2Class3AssistantName, record.StudentItem2Study, record.StudentItem2Description,
                record.StudentItem3Class3StudentName, record.StudentItem3Class3AssistantName, record.StudentItem3Study, record.StudentItem3Description,
                record.StudentItem4Class3StudentName, record.StudentItem4Class3AssistantName, record.StudentItem4Study, record.StudentItem4Description);
        }

        List<MSAHistoryItemStudent> BuildHistoryWeek_Students_Class(
            bool bSampleConversationVideo1,
            string bibleReadingStudent, string bibleReadingStudy,
            string studentItem1Student, string studentItem1Assistant, string studentItem1Study, string studentItem1Description,
            string studentItem2Student, string studentItem2Assistant, string studentItem2Study, string studentItem2Description,
            string studentItem3Student, string studentItem3Assistant, string studentItem3Study, string studentItem3Description,
            string studentItem4Student, string studentItem4Assistant, string studentItem4Study, string studentItem4Description)
        {
            List<MSAHistoryItemStudent> studentItems = new List<MSAHistoryItemStudent>();

            studentItems.Add(
                new MSAHistoryItemStudent
                {
                    Name = bibleReadingStudent,
                    Study = bibleReadingStudy,
                    Type = _transMSA.BibleReading
                });

            if (studentItem1Description != _trans.Video)
            {
                studentItems.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = studentItem1Student,
                        Assistant = studentItem1Assistant,
                        Study = studentItem1Study,
                        Type = studentItem1Description,
                        SampleConversationVideo = bSampleConversationVideo1
                    });
            }

            if (studentItem2Description != string.Empty)
            {
                studentItems.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = studentItem2Student,
                        Assistant = studentItem2Assistant,
                        Study = studentItem2Study,
                        Type = studentItem2Description
                    });
            }

            if (studentItem3Description != string.Empty)
            {
                studentItems.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = studentItem3Student,
                        Assistant = studentItem3Assistant,
                        Study = studentItem3Study,
                        Type = studentItem3Description
                    });
            }

            if (studentItem4Description != string.Empty)
            {
                studentItems.Add(
                    new MSAHistoryItemStudent
                    {
                        Name = studentItem4Student,
                        Assistant = studentItem4Assistant,
                        Study = studentItem4Study,
                        Type = studentItem4Description
                    });
            }

            return studentItems;
        }
    }
    catch (System.Exception ex)
    {
        SimpleLog.Log(ex);
    }
}

This looks very cluttered, you should remake CLMExplorerHistory to support multiple classes and multiple items in more organized way (eg. have a `List` where `StudentItem` contains `List`). Then, pass the object to the function instead of passing almost 20 strings. This would enable you to use indexes instead of variable names (`StudentItems[2]` instead of `StudentItems2`) — PawZaw, Jan 13 '22 at 07:59
@PawZaw I agree. Please see: https://stackoverflow.com/q/70684344/2287576 — Andrew Truckle, Jan 13 '22 at 09:13

Can this method be refactored to use a lamba?

1 Answers1

Linked