Extracting repeated for loop functionality into own method

user_10460880 0
Viewed 40

Within one of my classes I have a number of methods which all have similar purposes; these methods convert different objects into valid JSON representation. Each method does slightly different things as the objects being fed into the method are different and therefore their JSON output will also be subtly different.

Within these methods a for loop exists, the purpose of this loop is to check whether or not the field being converted into JSON is the last one in the object, if it is not then a , will be placed after the converted JSON string, as is normal within JSON.

Below is an example of one of these for loops:

         for (int i = 0; i < numberOfSections; i++)
            {
                if (i == numberOfSections - 1)
                {
                     output += SectionToJson(root.Sections[i]);
                }
                else
                {
                    output += SectionToJson(root.Sections[i]);
                    output += ",";
                }
            }

One thing to note here is that the call to the method (SectionToJson here) is different within each method. Therefore I have three different for loops doing almost the same thing but with different method calls inside their clauses.

I want to know whether or not there is a way that I can remove these ugly for loops from my three different methods and instead place their functionality inside a single method which can then be called from the three methods instead. However since the internal method call is different within each method, it makes it more difficult to place inside a single method.

I considered using the Func delegate to pass the required method through as a parameter to the new method, but this would not work as the parameters for the three internal methods are all different, and therefore I would need three different overrides of a single method. Which kinda defeats the point in removing the for loops in the first place.

Is there any other approach that I haven't considered that would help me achieve my goal here? I'm also trying to keep my list of parameters down, and would rather not go over three parameters in the new method. Preferably two.

The other two for loops in question are below.

for (int i = 0; i < numberOfQuestionsInBank; i++)
            {
                if (i == numberOfQuestionsInBank - 1)
                {
                    output += QuestionPropertyToJson(questionBank.Properties[i]);
                }
                else
                {
                    output += QuestionPropertyToJson(questionBank.Properties[i]);
                    output += ",";
                }

            }

for (int i = 0; i < numberOfSections; i++)
            {
                if (i == numberOfSections - 1)
                {
                    requiredSections += "\"" + (i+1) + "\"";
                }
                else
                {
                    requiredSections += "\"" + (i+1) + "\"";
                    requiredSections += ",";
                }


            }
c#
2 Answers

Well, there is another way - use string.Join this is exactly what it does.

And of course with linq you can make it look quite nice:

string.Join(",", root.Sections.Select(SectionToJson))

string.Join accepts collections of strings, so you keep concentrate on your conversion to strings (for each element) and let it do the concatenation for you.

user_13757530

The real-world solution to your problem is to use a JSON serializer, but of course this is an exercise so we can treat it a little differently.

What we need to do is look at the code an find the parts that are most alike. Once we locate that part of the code, we need to make it even more alike. Once they are identical, we can remove the duplication. They both remain 'til they're both the same.

So first of all, let's change both functions slightly to move the part that is different from the part that is almost the same. You can see that the if-statement in both is starting to look very similar.

        for (int i = 0; i < numberOfQuestionsInBank; i++)
        {
            output += QuestionPropertyToJson(questionBank.Properties[i]);

            if (i != numberOfQuestionsInBank - 1)
            {
                output += ",";
            }
        }

And

        for (int i = 0; i < numberOfSections; i++)
        {
            requiredSections += "\"" + (i + 1) + "\"";

            if (i != numberOfSections - 1)
            {
                requiredSections += ",";
            }
        }

Now let's consider what would happen if the naming was the same. Note: I'm not saying that you should make your naming the same everywhere as this would make your code less expressive - but we can do this exercise in our heads...

        for (int i = 0; i < recordCount; i++)
        {
            output += QuestionPropertyToJson(questionBank.Properties[i]);

            if (i != recordCount - 1)
            {
                output += ",";
            }
        }

And

        for (int i = 0; i < recordCount; i++)
        {
            output += "\"" + (i + 1) + "\"";

            if (i != recordCount - 1)
            {
                output += ",";
            }
        }

The section:

        if (i != recordCount - 1)
        {
            output += ",";
        }

Is now identical in both... we could create a function for that. There are a few ways to do that, this is just one of them:

    public string ConditionalComma(int recordCount, int i)
    {
        if (i != recordCount - 1)
        {
            return ",";
        }

        return string.Empty;
    }

That means our methods now look like this (I'll keep the in-head naming):

        for (int i = 0; i < recordCount; i++)
        {
            output += QuestionPropertyToJson(questionBank.Properties[i]) + ConditionalComma(recordCount, i);
        }

And

        for (int i = 0; i < recordCount; i++)
        {
            output += "\"" + (i + 1) + "\"" + ConditionalComma(recordCount, i);
        }

So we have managed to extract the differences and remove the duplication in a sensible way.

That's probably far enough for this exercise, but feel free to ask questions.

user_755250
Related