accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Drob" <md...@mdrob.com>
Subject Re: Review Request 19226: ACCUMULO-2477 Replace our implemenations of Entry
Date Fri, 14 Mar 2014 18:18:15 GMT


> On March 14, 2014, 6:04 p.m., Bill Havanki wrote:
> > core/src/main/java/org/apache/accumulo/core/data/KeyValue.java, lines 71-72
> > <https://reviews.apache.org/r/19226/diff/1/?file=519709#file519709line71>
> >
> >     The old toString separates the key and value by a space, but SimpleImmutableEntry
uses an equals sign. Now, I like equals better, but if anything is expecting the space for
parsing purposes, this change would be breaking.

KeyValue is not public API, so I'm not too concerned about breakage there. Besides, clients
shouldn't be doing string parsing anyway when programatic access is available. (Bloch, "Effective
Java SE" 53)

I checked our uses of it, and couldn't find any toString usage, outside of generating assertion
errors for tests.


- Mike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19226/#review37233
-----------------------------------------------------------


On March 14, 2014, 5:10 p.m., Mike Drob wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19226/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 5:10 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2477
>     https://issues.apache.org/jira/browse/ACCUMULO-2477
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-2477 Replace our implemenations of Entry
> 
> Most of our uses of Map.Entry can defer to SimpleImmutableEntry. We
> should use that instead of maintaining our own version.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java
30e12c1a42327d79dddf183992a1dc397607d407 
>   core/src/main/java/org/apache/accumulo/core/data/Key.java 74975aebb3cf1184f984d1325ccb9a02e23a6915

>   core/src/main/java/org/apache/accumulo/core/data/KeyValue.java 83996f86eb99cfa72867053a90f4370c1d41e4d7

>   core/src/main/java/org/apache/accumulo/core/util/Pair.java fb4ad80067cf5ea3a219a6b5876b6ed2f71fa8f3

>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TableToFile.java
3a211e2b55052292a7e28ed585c9848a81995a33 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java a454d6c1f3233120fa7af217afe5230542fed0c1

>   server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java dd20fd911e8e4bb047a35aec2bc94e5948e8075a

>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java cdb9dbf015be231358af420a79231082a9566ac0

> 
> Diff: https://reviews.apache.org/r/19226/diff/
> 
> 
> Testing
> -------
> 
> Existing unit tests still pass.
> 
> 
> Thanks,
> 
> Mike Drob
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message