The only downside is that the ArrayList now is always created, even if it is not needed, thereby producing unnecessary garbage. I have not bearing how frequently we'll get here and find no relevant regions, though.
-- Lars ----- Original Message ----- From: Harsh J <[email protected]> To: [email protected] Cc: Sent: Thursday, December 29, 2011 12:51 AM Subject: Re: question regarding code Yeah that'd work too. File a JIRA with the change? On 29-Dec-2011, at 2:12 PM, Mikael Sitruk wrote: > Hi all > > I have question on some code (taken from HLog) see below > > > static byte [][] findMemstoresWithEditsEqualOrOlderThan(final long > oldestWALseqid, > final Map<byte [], Long> regionsToSeqids) { > // This method is static so it can be unit tested the easier. > List<byte []> regions = null; > for (Map.Entry<byte [], Long> e: regionsToSeqids.entrySet()) { > if (e.getValue().longValue() <= oldestWALseqid) { > if (regions == null) regions = new ArrayList<byte []>(); > regions.add(e.getKey()); > } > } > return regions == null? > null: regions.toArray(new byte [][] {HConstants.EMPTY_BYTE_ARRAY}); > } > > Shouldn't be better to remove the if in the loop doing as follow? > > static byte [][] findMemstoresWithEditsEqualOrOlderThan(final long > oldestWALseqid, > final Map<byte [], Long> regionsToSeqids) { > // This method is static so it can be unit tested the easier. > List<byte []> regions = new ArrayList<byte []>(); > for (Map.Entry<byte [], Long> e: regionsToSeqids.entrySet()) { > if (e.getValue().longValue() <= oldestWALseqid) { > //if (regions == null) regions = new ArrayList<byte []>(); > regions.add(e.getKey()); > } > } > return regions.size() == 0? > null: regions.toArray(new byte [][] {HConstants.EMPTY_BYTE_ARRAY}); > } > > regards, > > Mikael.S
