1

Curious to know if there's a more elegant way of trying to find the sum of digits at odd positions in a string in Java 8.

This is currently my function

 /**
 * Explode string into char array, get sum of ASCII values at odd positions and divide by 10 to convert
 * to the integer value from that character
 * @param ccNumber string
 * @return int sum
 */
int calculateSumOfOddDigits(final String ccNumber) {
    final char[] digits = ccNumber.toCharArray();
    return (digits[0] + digits[2] + digits[4] + digits[6] + digits[8] + digits[10] + digits[12] + digits[14]) / 10;
}

Still not familiar with Streams and Java 8 and thought maybe you could do it like so:

 ccNumber.chars().mapToObj(x -> (char) x {
        ..add odd position digits

    })

Any suggestions welcome.

Stefan Zobel
  • 3,182
  • 7
  • 28
  • 38
tomaytotomato
  • 3,788
  • 16
  • 64
  • 119
  • How about starting from a plain non-stream loop? Or do you always expect your string to be exactly 15 character long? – RealSkeptic Mar 11 '17 at 21:57
  • have you tried simply just doing a for-loop? Maybe iterate through every odd character that way? – theKidOfArcrania Mar 11 '17 at 21:57
  • 2
    Is your code correct? it seems that you need to divide every character by 10. Also, Java `char`s are not in ascii encoding by default, you'd have to convert them first. – Jorn Vernee Mar 11 '17 at 22:00
  • Possible duplicate of [Take every nth element from a Java 8 stream](http://stackoverflow.com/questions/31602425/take-every-nth-element-from-a-java-8-stream) – MikaelF Mar 12 '17 at 01:49
  • @JornVernee dividing by 10 works for me and my unit tests, however you have got me worried now my code might not work in particular environments. – tomaytotomato Mar 12 '17 at 11:29
  • It is generally not a good idea to depend on the raw value of a `char`. The used encoding is an implementation detail, and has changed in the past. If you want ascii specifically, you can do `ccNumber.getBytes(StandardCharsets.US_ASCII)`, and use the resulting `byte[]` for the calculation. If that fails the tests, then what you have is probably the intended solution. – Jorn Vernee Mar 12 '17 at 11:37
  • 2
    @JornVernee Java `String` and `char` types are defined to be in UTF-16, the low range of which corresponds to ASCII. I have no idea what division by 10 is supposed to do though. – Stuart Marks Mar 12 '17 at 18:35

3 Answers3

9

What about something like this:

String str = "01234";
Integer sum = IntStream.range(0, str.length())
                       .filter(i -> i % 2 == 0)
                       .map(str::charAt)
                       .sum();
System.out.println(sum);
aioobe
  • 413,195
  • 112
  • 811
  • 826
Anton Balaniuc
  • 10,889
  • 1
  • 35
  • 53
  • I think it is generally bad practice to directly access data out of your streams context. It mainly doesn't scale. – YoYo Mar 12 '17 at 02:17
  • @YoYo: there is nothing wrong with accessing an immutable, random access data structure the way this answer does. Why does that not scale? In the current implementation (Java 8), it actually scales better than the original `chars()` implementation… – Holger Mar 13 '17 at 12:08
  • @Holger you use a stream for its laziness, and the possibility for parallelization. What if String is instead an 8 GB file? `Spliterator` cant come to the rescue - not like that. How can you switch over to Spark RDD - not like that. Nice implement, and suits this answer. It works is very unfortunate that there is no built-in that can generate a stream of (position,character) tuples. – YoYo Mar 16 '17 at 06:50
  • @YoYo: if you don’t have a randomly accessible data structure, you can’t access it. That doesn’t explain why accessing such a structure *when it is there* is “bad practice”. And it does scale; you can process strings of arbitrary length, as long as strings can get. If you replace the string with an 8GB file, you are not trying to scale, you are redefining the task. – Holger Mar 16 '17 at 09:54
  • @Holger Given the context, you are right. However, in that case, I do not find any benefit trying to express the solution of this problem in Java 8 Streams. Over a simple loop solution, arguably, readability has not improved. Then setting up and generating the lazy sequence comes for sure at an increased overhead. – YoYo Mar 16 '17 at 10:27
  • @YoYo: that heavily depends on the actual purpose of summing every second codepoint and dividing the sum by ten. Since I can’t imagine any reasonable use case, it’s not a question of readability anyway… – Holger Mar 16 '17 at 11:11
3

The problem is: if you want to filter on the index, it's not usefull to convert to a stream of chars, because you lose the index info.

You can, however, consider the indices as a stream, and use .map() to convert it to the char value. Something similar like this: Is there a concise way to iterate over a stream with indices in Java 8?

IntStream.range(0, ccNumber.length())
  .filter(i -> i % 2 == 0)
  .map(i -> ccNumber.charAt(i))
  .collect(Collectors.sum());

Not sure if this works exactly, but you get the idea.

Also: this is not nearly as efficient as a simple for-loop, so I wouldn't really bother to do it like this.

Community
  • 1
  • 1
GeertPt
  • 16,398
  • 2
  • 37
  • 61
2
Instream.range(0, str.length())
    .filter(i -> i % 2 == 0)
    .map(i -> str.charAt(i))
    .sum();
john16384
  • 7,800
  • 2
  • 30
  • 44