0

I have a recursive function in a windows service. This function upon completion rewinds itself as it has been repeated multiple times in recursion. Isn't that an overhead ?

Is there any way to avoid unwinding ? Is there any better approach?

Edit : In this method, I get 100 records from DB and then process them and then get another 100 and so on till all the records in DB have been processed.

Also, there is no limit of how many total records there might be in the db so this function can repeat itself quite a lot.

public void ServiceFunctionality()
{
    try
    {
        // Get Data From WEBAPI
        HttpClient client = new HttpClient();
        HttpResponseMessage response = response = client.GetAsync("webapi url link").Result;
        Response<ServiceWrapper> objResponse = response.Content.ReadAsAsync<Response<ServiceWrapper>>().Result;

        if (objResponse != null)
        {
            if (objResponse.isSuccess == true)
            {
                listContact = objResponse.data.lContact;
                int MaxPKinSelectedRecords = objResponse.data.MaxPKinSelectedRecords;
                int MaxPKinTotalRecords = objResponse.data.MaxPKinTotalRecords;

                if (listContact != null && listContact.Count>0)
                {
                    try
                    {
                        Parallel.ForEach(listContact, contact =>
                        {
                            // some code...
                        });
                        // Recursive Call
                        if (MaxPKinTotalRecords != MaxPKinSelectedRecords)
                        {
                            ServiceFunctionality();
                        }
                    }
                    catch (Exception ex)
                    {
                        // Logging
                    }
                }
            }
            else
            {
                // Logging
            }
        }
        else
        {
            // Logging
        }
    }
    catch (Exception ex)
    {
        // Logging
    }
} 
NewbieProgrammer
  • 874
  • 2
  • 18
  • 50
  • If you can make it tail-recursive, the compiler might make it more efficient. Or you could try to push some of the exception handling outside the recursion. – ryanyuyu May 14 '15 at 13:32
  • "Premature optimization is the root of all evil". If you really think it's a problem, run some tests timing the execution of your method. If your tests indicate it's a problem, then optimize. – mittmemo May 14 '15 at 13:33
  • 1
    @ryanyuyu - not in C# I'm afraid :( see http://stackoverflow.com/questions/491376/why-doesnt-net-c-optimize-for-tail-call-recursion – Steve May 14 '15 at 13:33
  • Can you use a while loop instead? – mageos May 14 '15 at 13:35
  • 5
    As an aside, I see no real need for recursion here. A simple loop would probably be better and prevent potential stack overflows. – GazTheDestroyer May 14 '15 at 13:35
  • [Every recursion can be converted to a loop.](http://stackoverflow.com/questions/931762/can-every-recursion-be-converted-into-iteration) – Zohar Peled May 14 '15 at 13:37
  • @GazTheDestroyer that knife cuts both ways, of course. "I see no need for looping here. If this won't overflow, simple recursion would probably be better" – Martijn May 14 '15 at 13:48
  • @Martijn: Yes, possibly, but IMHO a loop would be far more readable and easier to understand in this case too. – GazTheDestroyer May 14 '15 at 13:52
  • Thats rather subjective though. Personally, I tend to find recursive code easier to read, but generally write iteratively when needed as an optimization. – Martijn May 14 '15 at 13:53

2 Answers2

3

You can always unwind to a while loop. Because your calls aren't altering state, this is trival.

public void ServiceFunctionality()
{
    bool done = false;
    while(!done) {
    try
    {
        done = true; //if we don't reset this, we're done.
        // Get Data From WEBAPI
        HttpClient client = new HttpClient();
        HttpResponseMessage response = response = client.GetAsync("webapi url link").Result;
        Response<ServiceWrapper> objResponse = response.Content.ReadAsAsync<Response<ServiceWrapper>>().Result;

        if (objResponse != null)
        {
            if (objResponse.isSuccess == true)
            {
                listContact = objResponse.data.lContact;
                int MaxPKinSelectedRecords = objResponse.data.MaxPKinSelectedRecords;
                int MaxPKinTotalRecords = objResponse.data.MaxPKinTotalRecords;

                if (listContact != null && listContact.Count>0)
                {
                    try
                    {
                        Parallel.ForEach(listContact, contact =>
                        {
                            // some code...
                        });
                        // set loop variable
                        if (MaxPKinTotalRecords != MaxPKinSelectedRecords)
                        {
                            done = false;
                        }
                    }
                    catch (Exception ex)
                    {
                        // Logging
                    }
                }
            }
            else
            {
                // Logging
            }
        }
        else
        {
            // Logging
        }
    }
    catch (Exception ex)
    {
        // Logging
    }
} 
}
Martijn
  • 11,964
  • 12
  • 50
  • 96
1

Do not use recursion for calling a function whenever you have alternate suitable solution. I personally almost never do

I have tried to keep it same other than using a while..

Do not forget to break your loop. I tried to handle this thing but still

Just to be very careful, never take a risk of infinite loop on server I took maxPossibleIterations. So that in case of any mistake your web service server would not have to go for infinite iterations

public void ServiceFunctionality()
{
    long maxPossibleIterations = 999999;
    try
    {
        while (true)
        {
        maxPossibleIterations++;
        // Get Data From WEBAPI
        HttpClient client = new HttpClient();
        HttpResponseMessage response = response = client.GetAsync("webapi url link").Result;
        Response<ServiceWrapper> objResponse = response.Content.ReadAsAsync<Response<ServiceWrapper>>().Result;

        if (objResponse != null)
        {
            if (objResponse.isSuccess == true)
            {
                listContact = objResponse.data.lContact;
                int MaxPKinSelectedRecords = objResponse.data.MaxPKinSelectedRecords;
                int MaxPKinTotalRecords = objResponse.data.MaxPKinTotalRecords;

                if (listContact != null && listContact.Count>0)
                {
                    try
                    {
                        Parallel.ForEach(listContact, contact =>
                        {
                            // some code...
                        });
                        if (MaxPKinTotalRecords == MaxPKinSelectedRecords)
                        {
                            break;
                        }                        
                    }
                    catch (Exception ex)
                    {
                        // Logging
                    }
                }
                else
                    break; //Important
            }
            else
            {
                // Logging
                break;
            }
        }
        else
        {
            // Logging
            break;
        }
        } // End while
    }
    catch (Exception ex)
    {
        // Logging
    }
} 
Sami
  • 8,168
  • 9
  • 66
  • 99