Develop into Master: Finished challenges module 1 #1
Conversation
| int p1Selection = Integer.parseInt(player1.stream(). | ||
| sorted(Comparator.reverseOrder()). | ||
| map(Objects::toString). | ||
| limit(2). | ||
| reduce("", (s, n) -> s+n)); | ||
|
|
||
| int p2Selection = Integer.parseInt(player2.stream(). | ||
| sorted(Comparator.reverseOrder()). | ||
| map(Objects::toString). | ||
| limit(2). | ||
| reduce("", (s, n) -> s+n)); |
There was a problem hiding this comment.
Good use of streams, I wish there was an easier way to do "reduce with index" so that we could create the number without first converting it to a String and then parsing it
There was a problem hiding this comment.
Well, as a matter of fact there's a way to do the same using arithmetic and reduce with acc * 10 + b
| map(c -> switch (c.getType()){ | ||
| case "International" -> new CallSummary(c, c.getDuration() > 3 ? 22.68 + (c.getDuration() - 3) * 3.03 : 7.56 * c.getDuration()); | ||
| case "National" -> new CallSummary(c, c.getDuration() > 3 ? 3.60 + (c.getDuration() - 3) * 0.48 : 1.20 * c.getDuration()); | ||
| case "Local" -> new CallSummary(c, 0.20 * c.getDuration()); | ||
| default -> new CallSummary(c, 0.00); |
There was a problem hiding this comment.
nit: Similar comment as here
| String[] a = {h, m, s}; | ||
| for(int i = 0; i < 3; ++i){ | ||
| if (a[i].length() == 1){ | ||
| a[i] = "0" + a[i]; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: String.format could be a more reliable solution than adding the padding yourself
| int firstE = index; | ||
| if(index >= 5) firstE = index % 5; |
There was a problem hiding this comment.
nit: Even though we know that COUNTRY_NAMES has 5 elements, it's better to use .size(), your code would inadvertently break if we decided to add or remove elements from the COUNTRY_NAMES array, and this code wasn't properly tested this could potentially go undetected
| cont = cont.add(aux); | ||
| } | ||
|
|
||
| int l = (int) (Math.log10(cont.doubleValue()) + 1); //Obtaining length of the number |
There was a problem hiding this comment.
nit: Not that this is incorrect, but by using doubles we are entering a world of possible round errors, this may never happen, but a safer bet would be modulo to get the last n digits and then padding with zeros the string
| var res = facts[n]; | ||
| int l = (int) (Math.log10(res.doubleValue()) + 1); | ||
| int sum = 0; |
There was a problem hiding this comment.
nit: Same comment as here
| for(int i = 0; i < l; ++i){ | ||
| sum += res.mod(BigInteger.valueOf(10)).intValue(); | ||
| res = res.divide(BigInteger.valueOf(10)); | ||
| } |
There was a problem hiding this comment.
Good idea here, you could do a while instead of for to end the loop when your division reaches zero, i.e. we're already at the last digit
There was a problem hiding this comment.
Thank you!
I know a while would have been more intuitive instead of calculating a limit with the number of digits and using it in a for loop, but personally I prefer to use for before while, this because across the majority of programming languages, for loop are generally quicker than while loops.
I attach an interesting experiment someone did with java 17 to confirm this: https://tonisagrista.com/blog/2022/java-loop-performance/#java-17
There was a problem hiding this comment.
Well, you know you can also write for(;cond;)? And also, incurring into premature optimization could be a problem, we first write code that works and is readable and maintainable, then we care about optimization if needed.
| for(int i = 0; i < ascivalues.size() - 1; ++i){ | ||
| message.append((char)(c + ascivalues.get(i + 1))); | ||
| c = message.charAt(message.length() - 1); |
There was a problem hiding this comment.
nit: You could maybe improve how readable your code is here if you started with i = 1 until asciivalues.size (not inclusive)
There was a problem hiding this comment.
Yeah you are right, usually I see for loops like "number of times to iterate" so in my mind is like "Ok so I have to iterate all over this the number of times of the size of ascivalues, but I have already did the first one, so I have to subtract 1 to size"
Thanks for the comment, I just notice that from myself and the way I think until now XD
No description provided.