Skip to content

Commit 9a690fe

Browse files
thagikurathagikura
committed
Clean up code. (#314)
- Reuse the same instance for the FlexLinesResult instead of creating a new instance every time in the calculation of flex lines. - Increase the default allowing error in tests to reduce the false positive failures
1 parent e171891 commit 9a690fe

File tree

5 files changed

+105
-65
lines changed

5 files changed

+105
-65
lines changed

‎flexbox/src/androidTest/java/com/google/android/flexbox/FlexboxHelperTest.java‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ public void testCalculateHorizontalFlexLines() throws Throwable {
8181
.makeMeasureSpec(1000, View.MeasureSpec.UNSPECIFIED);
8282

8383
mFlexboxHelper.ensureIndexToFlexLine(mFlexContainer.getFlexItemCount());
84-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
85-
.calculateHorizontalFlexLines(widthMeasureSpec, heightMeasureSpec);
84+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
85+
mFlexboxHelper.calculateHorizontalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
8686

8787
assertEquals(3, result.mFlexLines.size());
8888
assertEquals(300, result.mFlexLines.get(0).getMainSize());
@@ -134,8 +134,8 @@ public void testCalculateVerticalFlexLines() throws Throwable {
134134
int heightMeasureSpec = View.MeasureSpec.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
135135

136136
mFlexboxHelper.ensureIndexToFlexLine(mFlexContainer.getFlexItemCount());
137-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
138-
.calculateVerticalFlexLines(widthMeasureSpec, heightMeasureSpec);
137+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
138+
mFlexboxHelper.calculateVerticalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
139139

140140
assertEquals(3, result.mFlexLines.size());
141141
assertEquals(300, result.mFlexLines.get(0).getMainSize());
@@ -188,8 +188,8 @@ public void testDetermineMainSize_direction_row_flexGrowSet() throws Throwable {
188188
int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
189189
int heightMeasureSpec = View.MeasureSpec
190190
.makeMeasureSpec(1000, View.MeasureSpec.UNSPECIFIED);
191-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
192-
.calculateHorizontalFlexLines(widthMeasureSpec, heightMeasureSpec);
191+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
192+
mFlexboxHelper.calculateHorizontalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
193193
mFlexContainer.setFlexLines(result.mFlexLines);
194194
mFlexboxHelper.determineMainSize(widthMeasureSpec, heightMeasureSpec);
195195

@@ -231,8 +231,8 @@ public void testDetermineMainSize_direction_column_flexGrowSet() throws Throwabl
231231
int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(1000, View.MeasureSpec.UNSPECIFIED);
232232
int heightMeasureSpec = View.MeasureSpec
233233
.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
234-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
235-
.calculateVerticalFlexLines(widthMeasureSpec, heightMeasureSpec);
234+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
235+
mFlexboxHelper.calculateVerticalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
236236
mFlexContainer.setFlexLines(result.mFlexLines);
237237
mFlexboxHelper.determineMainSize(widthMeasureSpec, heightMeasureSpec);
238238

@@ -272,8 +272,8 @@ public void testDetermineMainSize_direction_row_flexShrinkSet() throws Throwable
272272
int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
273273
int heightMeasureSpec = View.MeasureSpec
274274
.makeMeasureSpec(1000, View.MeasureSpec.UNSPECIFIED);
275-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
276-
.calculateHorizontalFlexLines(widthMeasureSpec, heightMeasureSpec);
275+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
276+
mFlexboxHelper.calculateVerticalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
277277
mFlexContainer.setFlexLines(result.mFlexLines);
278278
mFlexboxHelper.determineMainSize(widthMeasureSpec, heightMeasureSpec);
279279

@@ -313,8 +313,8 @@ public void testDetermineMainSize_direction_column_flexShrinkSet() throws Throwa
313313
int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(1000, View.MeasureSpec.UNSPECIFIED);
314314
int heightMeasureSpec = View.MeasureSpec
315315
.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
316-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
317-
.calculateVerticalFlexLines(widthMeasureSpec, heightMeasureSpec);
316+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
317+
mFlexboxHelper.calculateVerticalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
318318
mFlexContainer.setFlexLines(result.mFlexLines);
319319
mFlexboxHelper.determineMainSize(widthMeasureSpec, heightMeasureSpec);
320320

@@ -355,8 +355,8 @@ public void testDetermineCrossSize_direction_row_alignContent_stretch() throws T
355355
int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
356356
int heightMeasureSpec = View.MeasureSpec
357357
.makeMeasureSpec(1000, View.MeasureSpec.EXACTLY);
358-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
359-
.calculateHorizontalFlexLines(widthMeasureSpec, heightMeasureSpec);
358+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
359+
mFlexboxHelper.calculateHorizontalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
360360
mFlexContainer.setFlexLines(result.mFlexLines);
361361
mFlexboxHelper.determineMainSize(widthMeasureSpec, heightMeasureSpec);
362362
mFlexboxHelper
@@ -397,8 +397,8 @@ public void testDetermineCrossSize_direction_column_alignContent_stretch() throw
397397
int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(1000, View.MeasureSpec.EXACTLY);
398398
int heightMeasureSpec = View.MeasureSpec
399399
.makeMeasureSpec(500, View.MeasureSpec.EXACTLY);
400-
FlexboxHelper.FlexLinesResult result = mFlexboxHelper
401-
.calculateVerticalFlexLines(widthMeasureSpec, heightMeasureSpec);
400+
FlexboxHelper.FlexLinesResult result = new FlexboxHelper.FlexLinesResult();
401+
mFlexboxHelper.calculateVerticalFlexLines(result, widthMeasureSpec, heightMeasureSpec);
402402
mFlexContainer.setFlexLines(result.mFlexLines);
403403
mFlexboxHelper.determineMainSize(widthMeasureSpec, heightMeasureSpec);
404404
mFlexboxHelper

‎flexbox/src/androidTest/java/com/google/android/flexbox/test/IsEqualAllowingError.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class IsEqualAllowingError<T extends Number> extends BaseMatcher<T> {
3434
private Integer errorAllowed;
3535

3636
private IsEqualAllowingError(Number expected) {
37-
this(expected, 1);
37+
this(expected, 2);
3838
}
3939

4040
private IsEqualAllowingError(Number expected, int errorAllowed) {

‎flexbox/src/main/java/com/google/android/flexbox/FlexboxHelper.java‎

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,11 @@ private int[] sortOrdersIntoReorderedIndices(int childCount, List<Order> orders,
209209
* Calculate how many flex lines are needed in the flex container.
210210
* This method should calculate all the flex lines from the existing flex items.
211211
*
212-
* @see #calculateFlexLines(int, int, int, int, int, List)
212+
* @see #calculateFlexLines(FlexLinesResult, int, int, int, int, int, List)
213213
*/
214-
FlexLinesResult calculateHorizontalFlexLines(int widthMeasureSpec, int heightMeasureSpec) {
215-
return calculateFlexLines(widthMeasureSpec, heightMeasureSpec, Integer.MAX_VALUE,
214+
void calculateHorizontalFlexLines(FlexLinesResult result, int widthMeasureSpec,
215+
int heightMeasureSpec) {
216+
calculateFlexLines(result, widthMeasureSpec, heightMeasureSpec, Integer.MAX_VALUE,
216217
0, NO_POSITION, null);
217218
}
218219

@@ -221,6 +222,9 @@ FlexLinesResult calculateHorizontalFlexLines(int widthMeasureSpec, int heightMea
221222
* Stop calculating it if the calculated amount along the cross size reaches the argument
222223
* as the needsCalcAmount.
223224
*
225+
* @param result an instance of {@link FlexLinesResult} that is going to contain a
226+
* list of flex lines and the child state used by
227+
* {@link View#setMeasuredDimension(int, int)}.
224228
* @param widthMeasureSpec the width measure spec imposed by the flex container
225229
* @param heightMeasureSpec the height measure spec imposed by the flex container
226230
* @param needsCalcAmount the amount of pixels where flex line calculation should be stopped
@@ -233,10 +237,10 @@ FlexLinesResult calculateHorizontalFlexLines(int widthMeasureSpec, int heightMea
233237
* @param fromIndex the index of the child from which the calculation starts
234238
* @param existingLines If not null, calculated flex lines will be added to this instance
235239
*/
236-
FlexLinesResult calculateHorizontalFlexLines(int widthMeasureSpec,
240+
void calculateHorizontalFlexLines(FlexLinesResult result, int widthMeasureSpec,
237241
int heightMeasureSpec, int needsCalcAmount, int fromIndex,
238242
@Nullable List<FlexLine> existingLines) {
239-
return calculateFlexLines(widthMeasureSpec, heightMeasureSpec, needsCalcAmount,
243+
calculateFlexLines(result, widthMeasureSpec, heightMeasureSpec, needsCalcAmount,
240244
fromIndex, NO_POSITION, existingLines);
241245
}
242246

@@ -247,6 +251,9 @@ FlexLinesResult calculateHorizontalFlexLines(int widthMeasureSpec,
247251
* flex lines which includes the view who has the index as the {@code toIndex} argument.
248252
* (First calculate to the toIndex, then calculate the amount of pixels as needsCalcAmount)
249253
*
254+
* @param result an instance of {@link FlexLinesResult} that is going to contain a
255+
* list of flex lines and the child state used by
256+
* {@link View#setMeasuredDimension(int, int)}.
250257
* @param widthMeasureSpec the width measure spec imposed by the flex container
251258
* @param heightMeasureSpec the height measure spec imposed by the flex container
252259
* @param needsCalcAmount the amount of pixels where flex line calculation should be stopped
@@ -262,22 +269,25 @@ FlexLinesResult calculateHorizontalFlexLines(int widthMeasureSpec,
262269
* to the index, calculate the amount of pixels as the needsCalcAmount
263270
* argument in addition to that
264271
*/
265-
FlexLinesResult calculateHorizontalFlexLinesToIndex(int widthMeasureSpec, int heightMeasureSpec,
266-
int needsCalcAmount, int toIndex, List<FlexLine> existingLines) {
267-
return calculateFlexLines(widthMeasureSpec, heightMeasureSpec, needsCalcAmount,
272+
void calculateHorizontalFlexLinesToIndex(FlexLinesResult result, int widthMeasureSpec,
273+
int heightMeasureSpec, int needsCalcAmount, int toIndex, List<FlexLine> existingLines) {
274+
calculateFlexLines(result, widthMeasureSpec, heightMeasureSpec, needsCalcAmount,
268275
0, toIndex, existingLines);
269276
}
270277

271278
/**
272279
* Calculate how many flex lines are needed in the flex container.
273280
* This method should calculate all the flex lines from the existing flex items.
274281
*
282+
* @param result an instance of {@link FlexLinesResult} that is going to contain a
283+
* list of flex lines and the child state used by
284+
* {@link View#setMeasuredDimension(int, int)}.
275285
* @param widthMeasureSpec the width measure spec imposed by the flex container
276286
* @param heightMeasureSpec the height measure spec imposed by the flex container
277-
* @see #calculateFlexLines(int, int, int, int, int, List)
287+
* @see #calculateFlexLines(FlexLinesResult, int, int, int, int, int, List)
278288
*/
279-
FlexLinesResult calculateVerticalFlexLines(int widthMeasureSpec, int heightMeasureSpec) {
280-
return calculateFlexLines(heightMeasureSpec, widthMeasureSpec, Integer.MAX_VALUE,
289+
void calculateVerticalFlexLines(FlexLinesResult result, int widthMeasureSpec, int heightMeasureSpec) {
290+
calculateFlexLines(result, heightMeasureSpec, widthMeasureSpec, Integer.MAX_VALUE,
281291
0, NO_POSITION, null);
282292
}
283293

@@ -286,6 +296,9 @@ FlexLinesResult calculateVerticalFlexLines(int widthMeasureSpec, int heightMeasu
286296
* Stop calculating it if the calculated amount along the cross size reaches the argument
287297
* as the needsCalcAmount.
288298
*
299+
* @param result an instance of {@link FlexLinesResult} that is going to contain a
300+
* list of flex lines and the child state used by
301+
* {@link View#setMeasuredDimension(int, int)}.
289302
* @param widthMeasureSpec the width measure spec imposed by the flex container
290303
* @param heightMeasureSpec the height measure spec imposed by the flex container
291304
* @param needsCalcAmount the amount of pixels where flex line calculation should be stopped
@@ -298,10 +311,10 @@ FlexLinesResult calculateVerticalFlexLines(int widthMeasureSpec, int heightMeasu
298311
* @param fromIndex the index of the child from which the calculation starts
299312
* @param existingLines If not null, calculated flex lines will be added to this instance
300313
*/
301-
FlexLinesResult calculateVerticalFlexLines(int widthMeasureSpec,
314+
void calculateVerticalFlexLines(FlexLinesResult result, int widthMeasureSpec,
302315
int heightMeasureSpec, int needsCalcAmount, int fromIndex,
303316
@Nullable List<FlexLine> existingLines) {
304-
return calculateFlexLines(heightMeasureSpec, widthMeasureSpec, needsCalcAmount,
317+
calculateFlexLines(result, heightMeasureSpec, widthMeasureSpec, needsCalcAmount,
305318
fromIndex, NO_POSITION, existingLines);
306319
}
307320

@@ -312,6 +325,9 @@ FlexLinesResult calculateVerticalFlexLines(int widthMeasureSpec,
312325
* flex lines which includes the view who has the index as the {@code toIndex} argument.
313326
* (First calculate to the toIndex, then calculate the amount of pixels as needsCalcAmount)
314327
*
328+
* @param result an instance of {@link FlexLinesResult} that is going to contain a
329+
* list of flex lines and the child state used by
330+
* {@link View#setMeasuredDimension(int, int)}.
315331
* @param widthMeasureSpec the width measure spec imposed by the flex container
316332
* @param heightMeasureSpec the height measure spec imposed by the flex container
317333
* @param needsCalcAmount the amount of pixels where flex line calculation should be stopped
@@ -327,9 +343,9 @@ FlexLinesResult calculateVerticalFlexLines(int widthMeasureSpec,
327343
* to the index, calculate the amount of pixels as the needsCalcAmount
328344
* argument in addition to that
329345
*/
330-
FlexLinesResult calculateVerticalFlexLinesToIndex(int widthMeasureSpec, int heightMeasureSpec,
331-
int needsCalcAmount, int toIndex, List<FlexLine> existingLines) {
332-
return calculateFlexLines(heightMeasureSpec, widthMeasureSpec, needsCalcAmount,
346+
void calculateVerticalFlexLinesToIndex(FlexLinesResult result, int widthMeasureSpec,
347+
int heightMeasureSpec, int needsCalcAmount, int toIndex, List<FlexLine> existingLines) {
348+
calculateFlexLines(result, heightMeasureSpec, widthMeasureSpec, needsCalcAmount,
333349
0, toIndex, existingLines);
334350
}
335351

@@ -341,6 +357,9 @@ FlexLinesResult calculateVerticalFlexLinesToIndex(int widthMeasureSpec, int heig
341357
* attributes are done in a later procedure, so the views' measured width and measured
342358
* height may be changed in a later process.
343359
*
360+
* @param result an instance of {@link FlexLinesResult} that is going to contain a
361+
* list of flex lines and the child state used by
362+
* {@link View#setMeasuredDimension(int, int)}.
344363
* @param mainMeasureSpec the main axis measure spec imposed by the flex container,
345364
* width for horizontal direction, height otherwise
346365
* @param crossMeasureSpec the cross axis measure spec imposed by the flex container,
@@ -359,11 +378,9 @@ FlexLinesResult calculateVerticalFlexLinesToIndex(int widthMeasureSpec, int heig
359378
* to the index, calculate the amount of pixels as the needsCalcAmount
360379
* argument in addition to that
361380
* @param existingLines If not null, calculated flex lines will be added to this instance
362-
* @return an instance of {@link FlexLinesResult} that contains a list of flex lines and the
363-
* child state used by {@link View#setMeasuredDimension(int, int)}.
364381
*/
365-
FlexLinesResult calculateFlexLines(int mainMeasureSpec, int crossMeasureSpec,
366-
int needsCalcAmount, int fromIndex, int toIndex,
382+
void calculateFlexLines(FlexLinesResult result, int mainMeasureSpec,
383+
int crossMeasureSpec, int needsCalcAmount, int fromIndex, int toIndex,
367384
@Nullable List<FlexLine> existingLines) {
368385

369386
boolean isMainHorizontal = mFlexContainer.isMainAxisDirectionHorizontal();
@@ -380,7 +397,6 @@ FlexLinesResult calculateFlexLines(int mainMeasureSpec, int crossMeasureSpec,
380397
flexLines = existingLines;
381398
}
382399

383-
FlexLinesResult result = new FlexLinesResult();
384400
result.mFlexLines = flexLines;
385401

386402
boolean reachedToIndex = toIndex == NO_POSITION;
@@ -603,7 +619,6 @@ FlexLinesResult calculateFlexLines(int mainMeasureSpec, int crossMeasureSpec,
603619
}
604620

605621
result.mChildState = childState;
606-
return result;
607622
}
608623

609624
/**
@@ -1981,5 +1996,10 @@ static class FlexLinesResult {
19811996
List<FlexLine> mFlexLines;
19821997

19831998
int mChildState;
1999+
2000+
void reset() {
2001+
mFlexLines = null;
2002+
mChildState = 0;
2003+
}
19842004
}
19852005
}

0 commit comments

Comments
 (0)