1

My nested for loop looks like this:

for(int i=0; i<productGroups.size(); i++) {
     for(int j=0; j<productGroups.get(i).getProducts().size(); j++) { 
        AmountEntity storageValue = productGroups.get(i).getStorageValue();
        productGroups.get(i).getProducts().get(j).setQuantity(storageValue);
      }
}

I am pretty sure that there must be a more efficient way to do it without having n² complexity.

productGroup is a List which has Products Lists inside, and I am just traversing the both lists to set the amount.

I would be very happy if someone can help me here.

3
  • 2
    Taking into account the code you provided and what the logic seems to do, it seems it'll be O(n2) anyways -- or you could update the logic inside the class Product to return as quantity the value of storageValue-- ... as for code simplicity you could reduce it, maybe by using some lambda expressions Sep 17, 2021 at 8:43
  • 1
    You could move the line AmountEntity storageValue = ... in to the outer loop to save some time but otherwise the complexity will still be O(n^2).
    – Turamarth
    Sep 17, 2021 at 8:44
  • What List implementations do productGroups and products use? If it's e.g. a LinkedList, you get another hidden O(n), resulting in O(n³), from the get(i) calls. Sep 17, 2021 at 8:50

1 Answer 1

4

If you want to set a value on all products in all productGroups, you will need to visit each one. How many? Well if you have x productGroups and y products in each productGroup (for argument's sake) then you would need to visit x * y products. No way around that. You could make the code a bit more concise and readable though, which is usually way more important anyway:

for(ProductGroup productGroup : productGroups) {
    AmountEntity storageValue = productGroup.getStorageValue();
    for(Product product : productGroup.getProducts()) {
        product.setQuantity(storageValue);
    }
}

or in lambda form:

productGroups.forEach(pg -> {
    AmountEntity storageValue = pg.getStorageValue();
    pg.getProducts().forEach(p -> p.setQuantity(storageValue));
});

Note that each product in a productGroup gets the same storageValue so you can define that variable in the outer loop as shown above. If you can change the domain model, you could ask yourself why the storageValue needs to be copied from the productGroup to each product anyway since this is basically redundant information, but maybe there is a good reason for that.

If I saw logic like this I would not worry about the performance initially. Calling getters and setters is usually not a performance bottleneck. Only if you do this for millions and millions of products it might take some time. In that case the solution would not lie in speeding up these iterations, but in reducing the fetched number of products (e.g. by paging).

4
  • 1
    Yep ... or (arguably) ... productGroups.forEach(pg -> {AmountEntity storageValue = pg.getStorageValue(); pg.getProducts().forEach(p -> p.setQuantity(storageValue)); }); properly formatted in the source file Sep 17, 2021 at 8:56
  • "Well if you have i productGroups and j products in each productGroup (for argument's sake) then you would need to visit i x j products." This sentence is confusing; in the question, i and j are the indices, not the number of elements. Perhaps you could give different names, such as N and K, to the number of groups and number of products per group?
    – Stef
    Sep 17, 2021 at 9:08
  • Thanks Stef and lealceldeiro for you comments which I applied Sep 17, 2021 at 10:06
  • Thanks a lot everyone! I changed my code accordingly to your suggestions and pushed it for my senior engineers' code review! Thanks again for those really nice explanations! Sep 17, 2021 at 13:43

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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