• Post Reply Bookmark Topic Watch Topic
  • New Topic

Embedded for loop code improvement  RSS feed

 
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would need your advice to improve the following code because it takes so much time to execute method1 and method2. When I execute RemoveFullyContains I am calling method1 and method2. I have placed a time counter around both method and noticed that it takes so much time to execute both methods. May be someone can give me a guidance to improve it. Thank you


VG is a class that has the following methods: hashcode, equal and clone
 
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch!

When posting code, please UseCodeTags (←click that, it's a link)

When you say "so much time," exactly how much time are you talking about? "So much" is vague, relative, and subjective.
 
Jade Layyne
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello Junilu, thank you very much for your reply. It takes:
test method1 :  318 ms
test method2 :  30 ms
test method1 :  9730 ms
test method2 :  1153 ms
These results are in millisecond but as I have a lot of process before and after these lines of codes, I need to find a way to improve it even just for saving few millisecond. I have tried to replace the for loop in both method by doing something like that: if (!mainList.containsAll(searchList)) but the result is the same. Please let me know if you have an idea. Thank you
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, one thing I'd change in that design is the return value of those getter methods. If a method returns a list, don't allow it to return null, otherwise your client code will always have to do those checks for null.  A method that is declared to return a List should always return a list, even if it's just an empty list.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Your code is very deeply nested. Your for-loops go three levels deep in nesting. Each level of nesting increases the time complexity by an order of magnitude so if you want to make processing faster, you're going to have to find a way to limit the number of levels of nesting or make each more deeply nested loop complete as quickly as possible. Alternatively, you could try to find a different algorithm that didn't require so many levels of nested loops.
 
Jade Layyne
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Junilu, but how about the for loop?
 
Marshal
Posts: 56600
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:. . . Each level of nesting increases the time complexity . . .
It also increases the illegibility of the code, and makes it harder to work out what you are doing. The obfuscated method names don't help either. I think we can help if you explain very simply what you are trying to do.
Since you are new, I added the code tags, which I think you shou‍ld always use, to your code, and removed the colours because they can't be compiled. Doesn't it look better like that And welcome to the Ranch again.
 
Jade Layyne
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you very much Campbell
 
Campbell Ritchie
Marshal
Posts: 56600
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That's a pleasure But please tell us what you are trying to do; we can help a lot better if we know that.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jade Layyne wrote:I have tried to replace the for loop in both method by doing something like that: if (!mainList.containsAll(searchList)) but the result is the same. Please let me know if you have an idea.

If using containsAll() has about the same result, why would you choose to use the longer code instead of the more succinct containsAll()?

The above loop constructs are very non-idiomatic, that is, they are somewhat unusual. Why do you need to start at the end and work your way to the beginning of each list? And why do you initialize the inner loop variable in a way that makes it necessary to have that if statement? It would be more idiomatic if you did the following instead, and you can eliminate the if statement when you do it this way:

As far as squeezing out a few more millis of performance is concerned, I think other options for you are:
1. Figure out a different algorithm that doesn't have so many nested loops or
2. Sort your lists and use a search algorithm that is faster, like binary search.
3. If data input is slow, i.e., via keyboard rather than reading from a file, then you can do the tree trimming as lists are entered rather than as a "batch job"
 
Jade Layyne
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello everyone,
what I am trying to do is:
I have list of VG object (it is a VariableDataGroupData object). let us say:
I have 5 elements VG in my lTree list. VG0, VG1, VG2, VG3, VG4 objects. Each object (VG0, VG1, VG2, VG3, VG4) contains andVar String list and notVar String list.
method1 is actually the same as method2. It allows to check if list2 (the second parameter of method1 function)
contains list1 which is the first parameter of method1 function.
Know that list1 (the first parameter of method1 function) can be null or empty and list2(the second parameter of method1 function)
as well can be null or empty.
So if list1 is null or empty, method 1 return tru else if list2 is null or empty we return false.
Otherwise I need to check if list2 contains all elements of list1.
I need to compare each element of my list, with all other elements of my list except to the current element itself.
And if the current element if different from the other element:

If I start form the end of my list:
Let us say for (VG4 and VG3):
for x = 4 I need to compare (VG4 and VG3)  --> check if VG4.andVar contains VG3.andVar then check if VG3.notVar contains VG4.notVar, if both conditions are TRUe, the I remove VG4 from lTree list.
- if the current element is VG4: I need to check if (VG4.andVar string list contains all VG3.andVar string list(for that i need to check if both lists are not null or empty as I explained above))
if YES (TRUE) I will need to check if (VG3.notVar string list contains all VG4.notVar string list(for that i need to check if both lists are not null or empty as I explained above))
if YES (TRUE) I need to remove VG4 from my lTree list.
Then x = 3 I need to compare (VG3 and VG4) --> check if VG3.andVar contains VG4.andVar then check if VG4.notVar contains VG3.notVar, if both conditions are TRUe, the I remove VG3 from lTree list.
Then x = 2 I need to compare (VG2 and VG3).....
     x = 2 I need to compare (VG2 and VG4)
Then x = 1 I need to compare (VG1 and VG2)
     x = 1 I need to compare (VG1 and VG3)
     x = 1 I need to compare (VG1 and VG4)
Then x = 0 I need to compare (VG0 and VG1)
     x = 0 I need to compare (VG0 and VG2)
     x = 0 I need to compare (VG0 and VG3)
     x = 0 I need to compare (VG0 and VG4)
 
Campbell Ritchie
Marshal
Posts: 56600
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Whenever I see anything that complicated I start to think the design ought to be changed.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!