-1

I am part of a software team that is writing a console application (no UI) in C#. My part in the team is to write code to call RESTful APIs from a third party site, process the returned data and save it to the local database.

The third party API has a cap on the number of items returned in a single call (100 items max) which requires me to call the same API multiple times over a loop to retrieve all data which often runs into many thousands of records. Once a single record is retrieved, I need to make more API calls to get further details of the record that is retrieved which requires multiple loops again due to record cap imposed by the third party API.

All in all the process ended up being many levels of nested loops! I can't think of a better of way managing this process without so many nested loops. I thought about using lambda style expressions to enumerate items but I read somewhere that lambda expressions are syntactic sugar on loops! Any ideas that might improve the code without using nested, long painful loops?

  • 3
    > I read somewhere that lambda expressions are syntactic sugar on loops... I don't think this an accurate characterization of lambdas. Using higher order functions might help clean some things up, and, you don't have to use lambdas to use higher order functions. – Erik Eidt Oct 9 '18 at 18:06
  • 5
    From your question, it is not clear if you need to help to improve your code structure, or if you are looking for a way to reduce the number of resulting API calls. For the latter, we cannot actually help you without knowing anything about the API, and it may be possible that there might be no improvement possible. – Doc Brown Oct 9 '18 at 18:13
  • 1
    Ok, but I think it is still not clear what your specific problem is without seeing any of your code. Maybe you can extract a working code sample from your program and ask for feedback it on codereview.stackexchange? Just asking in general about "long loops" is not really much information about your issues. – Doc Brown Oct 9 '18 at 20:15
  • 2
    There's nothing wrong with loops. If you have a performance problem or untidy code, deal with it as such and refactor or implement caching. Don't aim to get rid of loops based on no reason at all. – JᴀʏMᴇᴇ Oct 10 '18 at 8:35
  • 2
    I have a feeling that if the API returned all the rows in one go, you would instead be complaining that either the API call, or the "sanitize, categorize and dump into a database" part, or both, were taking too long. Without knowing what performance requirements you have, or what the data is like, we can't really help you with that. From how you describe it, you don't have a problem, you just think you do. – Caleth Oct 10 '18 at 9:22
5

Loops are not necessarily bad. Lots of great features other than Lambda expressions probably compile down to loops. One of of the great things about computers is that it's easy to get them to repeat the same set of instructions as often as you need to, whether its by loops, recursion, lambda expressions etc.

What you probably do not want to do is have a bunch of complicated nested loops in the same function. Make each API call its own function, and have functions to handle building the data you're getting back, and the code should not be too ugly since you only care about a few particular loops at any one point in the process.

I would definitely not be afraid of Lambdas either. Many times they are more elegant than loops and if you are comfortable with them you should use them. Especially in a language like C# where they are powerful and popular.

Now if the problem is you are making too many API calls that are taking too long, or you get a limited number of API calls, then that is not really a problem with loops but with the architecture of getting your data.

|improve this answer|||||
  • Each API call is in its own function to process data returned by the call. My issue is not with API itself or my code. The code works just fine and I don't have issues with it. I was wondering if someone can enlighten me on alternatives to long loops. – Xami Yen Oct 9 '18 at 19:01
  • 2
    @XamiYen: To me there are two ways a loop can be "too long". One is that there is too much code in the loop and it has become difficult to understand what it does. If your code is well divided into modular functions with good names this should not be the case. The other issue is that the loop executes too many times or otherwise is too slow. It sounds like that is not true for you here either. Is there another specific problem with loops you want to avoid? – Nathanael Oct 9 '18 at 19:39
  • The code in the loop is not that much and is quite minimal. What I have is standard foreach loop with a bit of processing code to sanitize, categorize the returned data and dump into a database. The API returns only a 100 rows of data in each call, so I need to make multiple calls in a loop to fetch all data. Because of the recursive nature, the loop runs for a while. My reference to "long" is more to the number of times the loop runs than the time it takes to run which varies with each call. I am wondering how other developers might have coded such functionality better. – Xami Yen Oct 9 '18 at 23:27
  • 1
    Because of the recursive nature, the loop runs for a while. - so is performance your problem? As @Nathanael has said, outline your specific problem, or your idea of how you want to 'better' the loops. There is nothing inherently wrong with loops. – JᴀʏMᴇᴇ Oct 10 '18 at 8:34
1

The problem as I understand it: you have a API which implements paging (a limit on the number of items in a response) which is typical to prevent performance issues on both clients and servers. In order to loop over all the items, you need a loop for each response and a loop for calling the API repeatedly. In a nutshell, you have two loops where you want one.

Solution: create a method that returns an iterator. In C# you can use yield for this which greatly simplifies things. I don't code much C# but pythonesque psuedocode it would look something like this:

foo_items(params...):
  do:
    list items = call_api(params)
    for item in items:
      yield item
  while items != None

Now you can just loop over the entire iterator in the main part of your logic and hide the double-loop from view.

|improve this answer|||||
0

It's hard to tell what exactly the ideal solution is for the situation you are describing because I don't know all of the constraints. But generally the way I avoid nested loops situations like you are describing is to avoid trying to do everything at once and instead break things down into a more tiered structure. The end result is a series of medium sized loops rather than one big loop with many smaller loops inside it.

For example, let's say currently you have a main loop that finds all of the account IDs for the accounts you are looking for, and a nested loop inside that finds all of the event IDs for each account as you find them. Instead, you would make the first loop just get all of the account IDs and collect the IDs into a list. Then, once you get all of the IDs, make another loop to check each account in your list for event IDs. You can repeat this for more and more specific things. If the API calls are similar enough, you can probably share most of the actual code for calling the API multiple times and looping through getting the items, so that each "loop" step is just one call to API.GetAllRecords(type="Account") or whatever. Usually the more you break things down into separate steps that don't directly interact with each other, the more clean you can make your code.

|improve this answer|||||
  • I understand your point. Most of what you suggested is already being done except that I wrote recursive functions to make same API call to retrieve chunks of data due to data caps on the API itself. Once the data is returned fully, I pass the data to another function to make another API call to get more details of the records and so on. I am wondering if there is any better way. – Xami Yen Oct 9 '18 at 19:03
0

What about breaking up each loop into its own function? This allows you to encapsulate each level of abstraction. I think the issue is that (possibly for performance reasons?) you want to avoid looping through multiple lists. You'd rather just persist the data once you have it.

However, I would urge you to avoid this for 2 reasons: maintainability and readability. Your function sounds like it's doing too much by reaching out to the API, processing it, and saving it to the database all at the same time. Additionally, I'm guessing that you have not split your code into multiple levels of abstraction. This makes it difficult for a new programmer to begin looking at your code and dissecting what is actually happening.

What I recommend is splitting your function up. Plus, this allows your to see where the bottlenecks are. Then, if the performance isn't where you want it to be you can begin to refactor into something that handles the data more efficiently.

Your top-level function would then look like:

 Service.run() : void
     fetchAllRecordsFromAPI() : RecordList
     fetchAllRecordDetailsFromAPI(RecordList) : RecordDetailsList
     saveAllRecordsToDatabase(RecordList) : void
     saveAllRecordDetailsToDatabase(RecordDetailsList) : void

You would then implement each one of those functions to handle its specific, assigned function. Obviously it's pseudocode and it's not as clean as it could be, but you get the point.

|improve this answer|||||

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