Yeah I think we should change it. Does not look like it's going to be called often enough to make a difference. So better readability is good :)
Harsh J <[email protected]> schrieb: >Lars, > >Yes, but that vs. unnecessary if-checks in every loop, makes it okay? >Unless these are little nitpicky worries and JVMs do a lot better >magic inside :) > >On Thu, Dec 29, 2011 at 10:24 PM, lars hofhansl <[email protected]> wrote: >> 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 > > > >-- >Harsh J
