Limiting amount of code and switch statements

Question

0.00/5 (No votes)

See more:

Hi All,
i was hoping for help with an issue i'm having with regards to cutting down repetitive code.

i am wanting to write a method for a dynamic page, that takes in a unknown object (possible 4 options).

It then creates a new instance of that type of object and loads all properties (done in object constructor). But need every posibiility to tell it which objects related objects to load, e.g if product loop through product.RelatedContent, if industry loop through industry.RelatedContent if you understand what i mean.

Then access a list property of that object to access subObjects to build html based on those details. But rather than writing a switch case, for every posibility and thus causing smelly code, and repeating myself, is there anyway i can write the code to be more generic and loosely coupled? current code (i've only done 1 example save mass code):

C#

public partial class RelatedContent : System.Web.UI.Page
{
    Product prod;
    KnowHowArea kha;
    Khia khia;
    Industry ind;
    Belzona.ILanguage language;

    protected void Page_Load(object sender, EventArgs e)
    {
        var referer =  Request.QueryString["Referer"];
        var request = Request.QueryString["Request"];
        var number = Request.QueryString["Number"];
        language = Belzona.Language.GetByUIValue(Request.QueryString["Language"]);

        LoadReferer(referer, number,request);
    }

    private void LoadReferer(string referer, string number,string request)
    {
        StringBuilder sb = new StringBuilder();
        Guid guid = Guid.Parse(number);
        sb.Append("<ul id=\"i_lst\" data-role=\"listview\" class=\"related_links_nested\">");

        switch (referer)
        {
            case "prod":
                prod = new Belzona.ProductsMySQL.Product(number);
                switch (request)
                {
                    case "ind":
                        foreach (IndustryProxy ind in prod.RelatedIndustries)
                        {
                            sb.Append("<li class=\"ui-li-has-thumb ui-btn ui-btn-icon-right ui-li ui-btn-up-c\">");
                            sb.Append("<a data-transition=\"slide\" href=\"industry_info.aspx?guid=" + number + "&language=" + language.UIValue + "\">");
                        }
                        break;
                    case "khia":
                        foreach (Khia khia in prod.RelatedKnowHowInActions)
                        {
                            sb.Append("<li class=\"ui-li-has-thumb ui-btn ui-btn-icon-right ui-li ui-btn-up-c\">");
                            sb.Append("<a data-transition=\"slide\" href=\"khia_info.aspx?guid=" + number + "&language=" + language.UIValue + "\">");
                        }
                        break;
                    case "app":
                        foreach (Belzona.KnowHowAreas.KnowHowAreaProxy knowHow in prod.RelatedKnowHowAreas)
                        {
                            sb.Append("<li class=\"ui-li-has-thumb ui-btn ui-btn-icon-right ui-li ui-btn-up-c\">");
                            sb.Append("<a data-transition=\"slide\" href=\"khia_info.aspx?guid=" + number + "&language=" + language.UIValue + "\">");
                        }
                        break;
                }
                break;
            case "ind":

                ind = new Belzona.Industries.Industry(guid, language);
                break;
            case "khia":
                int id = int.Parse(number);
                khia = new Belzona.KnowHowInAction.Khia(id);
                break;
            case "app":
                kha = new Belzona.KnowHowAreas.KnowHowArea(guid, language);
                break;
        }
    }

Posted 24-Nov-14 5:23am

Grant Weatherston

Add a Solution

Comments

Afzaal Ahmad Zeeshan 24-Nov-14 11:27am

They'll be same even if you used an if block, since there is no similarity and you're going to need all these blocks as they seem to be a condition-like something.

Grant Weatherston 24-Nov-14 11:28am

so what you're saying is either way have to have duplicate code?

Afzaal Ahmad Zeeshan 24-Nov-14 11:40am

This is not duplicate, you can see that the condition is different and the code in them is also different.

Grant Weatherston 24-Nov-14 11:42am

but the other 3 outer case statements will have the exact same inner code, with the objectname changed a little

Afzaal Ahmad Zeeshan 24-Nov-14 11:56am

Have a look at my answer, I have changed that block into a loop for the total elements since it was similar. :)

- also, why are you even adding a new list item each time you execute the loop, and not closing that element at all?

BillWoodruff 24-Nov-14 14:37pm

There's a lot you could do to refactor this code to make it more readable and maintainable, as well as a little more memory efficient. But, Afzaal is correct in his assertion that the essential logic required is not going to change.

In the case of processing a Product I note that the code executed for both 'khia and 'app in the foreach loop is identical: is that correct ?

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

BillWoodruff · Answer 1 · 2014-11-24T10:03:00

I am averse to refactoring code when that's part of my consulting practice, but I do want to give you some suggestions:

1. more Generic ? For this I think that 'ind 'khia, and 'app objects would have to all implement a common Interface that you could program with/to, to make a kind of "class factory" with some "sockets" in it for injection of methods, or other objects. hard to say more without really knowing the overall data-structures and Classes here.

2. yes, I'd separate out the handling of the 'prod result from the other three cases, put it in its own Method.

3. re-use the StringBuilder: use the Clear method (available in .NET 4.0 and later) to clear it before using it again.

4. move the repeated strings out of the 'switch statements and make them string Constants, and re-use the references to them to create cleaner, more readable code.

Afzaal Ahmad Zeeshan · Answer 2 · 2014-11-24T05:54:00

As I already told you that there is no way you can minimize the block, or the switch usage you're going to use them because they're all different conditions. Until you posted that comment - which was helpfull and here would be the block for your application.

Inside the first block, you're having three code blocks which perform the same activity but their loop depends on the number of elements in each three collections. So what you can do is, to just count the total of them and loop (for can be used) and add the code block in it.

C#

int totalLoopCounts = prod.RelatedIndustries.Count + 
                      prod.RelatedKnowHowInActions.Count + 
                      prod.RelatedKnowHowAreas.Count;
// if Count is not available, use Count(), but Count should work to count the total
// Run the loop until it completes the process for all of the elements
for (int i = 0; i < totalLoopsCounts; i++)
{
    // for each of the item, loop and execute the following
    sb.Append("<li class="\"ui-li-has-thumb" ui-btn="" ui-btn-icon-right="" ui-li="" mode="hold" />    sb.Append("<a data-transition="\"slide\"" href="\"industry_info.aspx?guid="" number="" language.uivalue="">");
}
</a>

Add the above code to the first switch below the

C#

case "prod":
    prod = new Belzona.ProductsMySQL.Product(number);

Tip: Why are you adding a new list item without closing the previous one? Fix that one please.

Limiting amount of code and switch statements

2 solutions

Solution 2

Solution 1

Add your solution here

Preview 0