Stephan van Hulst

I'll tell you the same thing I've told Liutauras. If you create your work on separate branches and then make pull requests when you want your code to be reviewed, it's much easier to review and keep the comments in one place.

Let us know what commit on master you want to use as a baseline for an initial review.

If that were true, I would never get any work done.

I often use apps such as Postman or Advanced Rest Client to test my back-ends, or other tools such as Fiddler or sometimes even Wireshark if I'm really deep down the rabbit hole.

Mike Simmons wrote:Aside from the extra collection, I'm a bit troubled by doing all the summation on floating-point values.  Seems like it would be less error prone to do the summation on longs, with nice precise values, and then divide by the observation count at the end.  In this way you can better guarantee that the total probability will be 1.0, rather than 0.999999999756 or some such.

Strongly agreed. Always perform division as late as possible, and then prefer BigDecimal over Double, unless performance is important and accuracy isn't.

In several places, we could also replace List and TreeMap with more general types like Collection and NavigableMap.  But I get used to the short versions of each, even when they're unnecessarily specific.

Please get into the habit of doing this, especially when creating public method signatures. Many APIs are awkward to use or maintain because they contain types that are more specific than required.

Here are the tweaks I came up with, for your consideration:

Don't use raw types. Declare your type variable as T extends Comparable<? super T>.

Use wildcards in method parameters: Map<? extends T, ? extends Long>. I don't feel strongly about doing this with final types (like Long), but it is more consistent and clearly conveys intent.

Why is the size parameter a double? If the only reason is to perform an implicit cast, then you're sacrificing the principle of least astonishment to make your code just a tiny little bit less verbose.

Why are you iterating over a copy of the map? You're not modifying the original map, so iterating over it is safe. If the reason is that you need the map to be sorted, then just accept a sorted map.

Personally I would have your remap() method operate on the current map, rather than a new one. It saves a lot of copying, and if you actually need a copy then just make the copy before you pass it into the method.

Don't use the return values of assignments in enclosing expressions in any but the most common idioms, such as incrementing an index while accessing an array.

My attempt:
static <T extends Comparable<? super T>> Collector<T, ?, NavigableMap<T, BigDecimal>> accumulatingDistribution(MathContext context) {   final class Accumulation {     private final NavigableMap<T, BigDecimal> counts = new TreeMap<>();     private long total;     private void accept(T observation) {       counts.merge(observation, ONE, BigDecimal::add);       total++;     }     private Accumulation combine(Accumulation other) {       other.counts.forEach((observation, count) -> this.counts.merge(observation, count, BigDecimal::add));       this.total += other.total;       return this;     }     private NavigableMap<T, BigDecimal> finish() {       BigDecimal      totalCount = BigDecimal.valueOf(total);       BigDecimal cumulativeCount = ZERO;       for (Entry<T, BigDecimal> entry: counts.entrySet()) {         cumulativeCount = cumulativeCount.add(entry.getValue());         entry.setValue(cumulativeCount.divide(totalCount, context));       }       return counts;     }   }   return Collector.of(Accumulation::new, Accumulation::accept, Accumulation::combine, Accumulation::finish); }

Piet Souris wrote:thanks for the critiques, even if it is not very egoless!

If, going by the description of peer reviews that I found on the Wiki for egoless programming, you mean that my review wasn't friendly or collegial, I apologize. I try to write in a neutral way but that doesn't always come across through text.

As you see, it is just a very clear short one-liner.

Short yes, clear no. I had to look twice because the combination of the forEach() and the multiple statements on one line first made it look like a regular for loop. I would have taken your point about the forEach being a nice construct to use over an enhanced for-loop, if you had named the variables k and v something descriptive like observation and count, but now you've only used it for the sake of brevity.

Hmm, cdf is a very much used abbreviation, so for me it was very clear.

But as soon as you posted your code for review, it was for a wider audience than just yourself. I always recommend writing identifiers out in full, so there's as little ambiguity as possible. Avoid abbreviations or symbols common to math, unless they are REALLY common, such as constants like PI. Also note that as a method name, cdf is redundant. It's like I suffixed the word "method" to all my method names.

Correct, but for now I limit myself to numeric random variables, with their in-built natural ordering.

Why? Also, in software development, "for now" is never "for now".

Can you give us a small program that we can compile and run that illustrates what you've described?

I find it really strange that you like using Java's functional features so much Piet, but then shirk the 'proper way' of doing things in preference of using hacks like modifying objects to get around the limitation that you can't modify local variables from lambdas, which is there for a good reason.

The forEach() in your cdf2() method is not functional. It's procedural code. Then why not just use a for-each loop? That way you won't even have to resort to silly things like using the array.

Why is your CDF keyed by probability? A CDF returns a probability given an observation, so it makes sense to keep T as the key.

You use two collect() operations in your cdf() method. That's wasteful. Use a downstream collector to specify what should be done with the values of the map while the groupingBy() is doing its magic.

Why do you multiply by 1.0 to cast your value? That's what the (double) operator is for.

Don't repeat method calls inside loops. Save calls like observations.size() to local variables first.

When you split up methods into smaller chunks (a good thing) make sure to give the chunks descriptive names. I'd call cdf() getCumulativeDistribution() and I'd call cdf2() accumulateDistribution().

Why is T a Number? The only thing that your methods require about T is that it has a natural order.

In previous examples of your XML I noticed that you have the n1 prefix defined in both the root element and the element that is giving you problems. My guess is that the XML that is giving you issues doesn't have the redefinition in the pdBranchCustomerNotification element, so it uses the definition from the root element.

Sounds like the author made a lot of assumptions that are not inherently true for all applications of those technologies, just how they are often (wrongly) used.

Wrong and wrong.

You can configure how the login form acts. Also, there's nothing inherently unsafe about cookies.

Basic Auth doesn't specify how the user must be remembered, just how the credentials are sent in the first place.

How did you get your assumptions?

Strings naturally compare lexicographically. You can just put them in a list and sort the list. If you want to know which of two strings is larger, you can use String.compareTo().

In general it's a good idea to farm out authentication. Why store user credentials if you can let someone else do it?

For a social network site I can imagine you don't want to connect it to a competing one, but it's not unheard of.

Anyway, you can use HTTP Basic and let the browser handle the login prompt, or you can just create your own login form and send the credentials like you would in any other POST request.

I'm not sure why you want to use OAuth if you intend to store the user credentials yourself. OAuth has a "Resource Owner Password Credentials" flow, but it's only intended to slowly migrate an existing application from a username/password form to authentication through a third party.

If you're using Spring, you might want to add Spring Security and define a PasswordEncoder bean to safely hash and verify a user password. Then accept the user's credentials through a custom end point. Make sure to use POST, so the credentials can be sent in the body of an encrypted request.

Michael Stiefler wrote:...and default methods since Java 8.

Errr... yes. Oversight on my part.

Do users need to provide their credentials somewhere or does your client application have a "hardcoded" secret with which it authenticates itself without the user having to do anything?

So far it sounds like the guy has absolutely no clue. Forget that video.

No, you don't need PHP and JavaScript for social media.

No, Spring is not just for banks.

No, JavaScipt and Spring aren't mutually exclusive.

Spring can easily be used as a back-end for a social media sites, with or without JavaScript to make the front-end more dynamic.

Have you ever written a web service using a plain old Java EE servlet? You will find out after you've written a moderately complex service (with a database and multiple types of requests that a user can make), how much work Spring takes out of your hands for the same application.

I'll see if I can write two versions for you to compare later this week, but I also suggest you just try it out.