DroidKaigi / conference-app-2017

The Official Conference App for DroidKaigi 2017 Tokyo
Apache License 2.0
470 stars 140 forks source link

Using the 'for' statement instead of create new Stream instance. #366

Closed kitwtnb closed 7 years ago

kitwtnb commented 7 years ago

Issue

Overview (Required)

Links

konifar commented 7 years ago

Thanks!! LGTM 💯

eneim commented 7 years ago

@konifar @kitwtnb As reference for this blog post, I suspect that this fix gives same result: a new instance of Iterator is implicitly allocated for each iterator for loop call. My suggestion is to use the for loop using index (which simply get the instance from the list reference, no new thing is allocated). Your code looks like 1990 java but ...

P/S: sorry for being a bit OCD >.<. But 60fps FTW :p.

konifar commented 7 years ago

Sorry, Let me confirm your opinion 🤔 You mean following code is better?

for (int i = 0, size = particles.size(); i < size; i++) {
    particles.get(i).draw(canvas, paint);
}

List<Line> lines = createLines(particles);
for (int i = 0, size = lines.size(); i < size; i++) {
    lines.get(i).draw(canvas, paint);
}
eneim commented 7 years ago

@konifar Yes. The point is to prevent object allocation in onDraw, and Iterator by for (Object item: items) will implicitly create one (this is compiler work, we can do nothing).

The second part is to produce new lines for the particle lis, but I think is fine now, so just reduce the allocation as much as possible :D.