Clean up imports and whitespace. #88
Conversation
| @@ -99,7 +99,7 @@ public Contig apply(String contigString) { | |||
| // the ShardUtils methods should be used to ensure that shards are shuffled all together before | |||
| // being returned to clients. | |||
| List<Contig> getShards(long numberOfBasesPerShard) { | |||
| double shardCount = Math.ceil(end - start) / numberOfBasesPerShard; | |||
| double shardCount = (end - start) / (double) numberOfBasesPerShard; | |||
There was a problem hiding this comment.
You can just keep them as long - the double has no effect, as the decimal portion and will not have impact in the for loop. Any decimal portion in a long will be just Math.floor automatically.
There was a problem hiding this comment.
Math.floor isn't what we want here. See also the unit tests.
The useless Math.ceil there before caused a double cast on the numerator, which is why the old version "worked".
There was a problem hiding this comment.
@pgrosu please see https://github.com/googlegenomics/utils-java/blob/master/CONTRIBUTING.rst regarding code
There was a problem hiding this comment.
Thanks Nicole, but I am not contributing code, and not sure why my post just disappeared though. This is a friendly discussion of exploratory ideas, and the code I posted was mostly your code with a proof-of-concept of why Math.ceil might not be useless. This is just Java-logic that commonly discussed everywhere among all programmers.
|
LGTM - that's a lot of whitespace. |
This is in preparation for v1beta2 -> v1 code changes.