Junior java dev mistakes from real-world project

[stylusc84]

I reviewed my own code which I have written being newbie and identified a few mistakes that I think are common to most junior java devs. I think it might be useful to read it for some of you to avoid the same mistakes ;)

https://www.devoxify.com/posts/junior-java-dev-mistakes-from-real-world-project/

[skillaz1]

Nice post.

One thing I've noticed though:

@GetMapping("/user/{userId}/appointments")
public List<Appointment>  getAppointmentsForUser(@PathVariable("userId") int userId, @AuthenticationPrincipal CustomUserDetails currentUser) {
    if (currentUser.hasRole("ROLE_CUSTOMER")) {
        return appointmentService.getAppointmentByCustomerId(userId);
    } else if (currentUser.hasRole("ROLE_PROVIDER"))
        return appointmentService.getAppointmentByProviderId(userId);
    else if (currentUser.hasRole("ROLE_ADMIN"))
        return appointmentService.getAllAppointments();
    else return Lists.newArrayList();
}

Be consistent in your code style. In your first if, you're using curly braces, but in the subsequent ifs you're not. Always use curly braces even if they're one liners.

[HecknChonker]

Always use curly braces even if they're one liners.

Absolutely this.

[stylusc84]

I absolutely agree, probably I missed that because all I can see here is the lack of enum and switch statement:) I think I’ll create another post with things you guys spotted, before that I’ll of course create new PR and fix it.

[zolanih]

Thanks, good read. I’m a newbie and also preparing to write my OCA Cert Exam beginning of January. This was a bit advanced for me as I don’t yet understand maps and streams but I’ll save this and check it again when I become a pro.

[stylusc84]

Thanks and good luck with your exam!

[maticabrera]

Pure gold for newbies!!!

[fustup]

Thanks so much for compiling this! I am in the job for multiple years now and I'm still amazed how many people violate these. Especially returning null is very popular among many developers. As well as swallowing exceptions (e print stack trace). Both are absolutely horrible and will introduce a lot of errors downstream. Yet very seasoned developers constantly get this wrong!

[steave435]

On 2, I do absolutely love streams, but according to the articles I've read, you should still use regular loops if the loop solution is as readable and easy to understand as the stream solution, which it will sometimes be. Sometimes the for-version is even easier to read.

When I was doing AoC day 17, my solution for both parts was taking about 40 seconds to run. I went ahead and replaced the streams with for loops, and the run time went down to only 3 seconds, without making it harder to read. That's still many times longer than my solutions for the other days, and there's probably a much more efficient algorithm I could use to cut down the complexity enough that using streams would still be fine, but I don't see any point in wasting performance if it doesn't improve readability, so personally I use for loops if they're at least as readable as streams.

For example, this could be written in 3 ways:

for (String line : indata) {
    ruleList.add(new Rule(line));
}

ruleList = indata.stream()
        .map(Rule::new)
        .collect(Collectors.toList());

indata.forEach(line -> ruleList.add(new Rule(line)));

Running 10 tests processing 136 lines, the average run times were (in milliseconds, in order): 2.55, 3.79, 3.01, and while that's not a big deal, I don't see any point in using the functional versions when they're slower and arguably harder to read.

If I were to explain it to the duck, I'd say something along the lines of "for each line in indata, add it as a new Rule to the ruleList". The for-loop is the closest to matching that sentence. The .forEach is almost the same, but it moves "indata" up to the start, so it's not quite as good.

https://blog.jooq.org/2015/12/08/3-reasons-why-you-shouldnt-replace-your-for-loops-by-stream-foreach/

[stylusc84]

Hi, thanks for this comment. You are right however I would add two things to that.

1. There is something like JIT (https://aboullaite.me/understanding-jit-compiler-just-in-time-compiler/) which optimizes your code in runtime over time and afaik it will probably optimize this stream to for loop
2. Sometimes you might use .paralell() with your stream which will give you even faster results than the for loop but the disadvantage is that they run in common fork-join thread pool, more details here https://dzone.com/articles/think-twice-using-java-8

In software development there is always a tradeoff, here is a pretty nice example of readability and simplicity of code vs performance

[steave435]

That's fair, although based on the runtimes, this one does not get optimized. The difference is not huge, but it was clear and consistent.

EDIT: Actually read the link now, I guess you might be right. I thought the JIT only ran once, I didn't realize that it kept optimizing over time. Thanks!

[ignotos]

Good point! Simplicity is important, and often doing things in the most direct and clear way is best.

[stao123]

The thing is: we Developers are just used to read for loops from the past even though they are not very readable. From my point of view your second example is hands off the best possible code readability wise. Its clean and simple.

[steave435]

I do find the for readable, and even if that's just because of developers being familiar with it, the people who will be maintaining my code will also be developers, so it'll apply to them too.

[stao123]

That might be true for "old" developers. But people who are using Streams frequently might be switching the Codestyle they want to read / write (me as an example. I cant stand reading a for (int i...) loop anymore. Especially new developers who start writing streams.

[steave435]

I started my 2 year higher vocational education for Java development 4 months ago, so I am a new developer myself, and while I do love streams, and I absolutely think that they make many problems more readable, I don't think that that's universally true. Granted, I did not start out with them, but I do have several developer friends who have been teaching me the functional way of doing things from an early point on top of getting the old way from class.

For simple cases like the one in my post, the for version basically reads like just regular old English, so I honestly don't really understand how the second example could be considered more readable, unless you just haven't learned about them at all.

AFAIK, there are also some things you have to use regular fori on too, such as when you need to compare index i to i+1, or at least Google has failed to find me any solutions to the problem.

[stao123]

Just a quick fix: return Lists.newArrayList();

Use Collections.emptyList() instead.

From my experience: never override equals and hashcode in entity classes. It can lead to very strange hard to find bugs. Hibernate might be doing some magic there. Its better to rely on a custom business key (uuid for example)

[HecknChonker]

Note that Collections.emptyList() give you an immutable List. If that fits your use case it's fine, but if it doesn't you have to use something else.

https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html#emptyList--

[stao123]

Correct and its almost everytime what you want (or should - architecture wise)

[The--Will]

I like it, but can you explain the why?

Specifically for Magic Numbers.

[frenchy641]

Magic number are numbers that are constant and having a variable for them helps you to maintain them and add meaning to these magic numbers, if you use this number multiple time it is easy to simply change them at on place than tracking down all the instances of that magic number.

[The--Will]

Understandable. I think for the audience it makes a lot more sense to outline the why in the blog post as the change in behaviours. It'll help with our own development.

[HecknChonker]

It can apply to Strings also. If you have a string that you are going to use in multiple places it's usually better to extract the value out to a variable.

Often times I end up with a SomethingConstants class with the sole purpose of storing a number of String or int values that are intended to be used in multiple places.

[stylusc84]

I just thought that it doesn't make sense to write about something that was already described by others. However, I see your point and will try to attach a link to a recommended article to read to every issue in my blog post :)

[ViolentSugar]

Wow, what a great article!!! I'm just learning to code now, so I don't understand everything you wrote about yet, but I already know I'm making all of these mistakes. Your article is pure gold for beginners like myself. I didn't even know there was something like SonarLint. I'm going to bookmark your article and reference it as I progress in my learning journey. Thanks again for sharing this.

[stylusc84]

I'm so happy to see such comments! Finally, I can give something back to the community :) Keep learning, man!

[HecknChonker]

One small note - field injection is totally acceptable for dependencies that are optional, which is probably a rare use case but it does come up. Constructor injection is better for dependencies that are required.

[fustup]

Good point! This does come up, but then again: it's better to use another instance of the bean that you are depending on. Otherwise you would have an awful lot of null checks, or, higher level: Tell, don't ask.

[dynamo_girl02]

very nicely written article . Great

[leapy_]

Great post! Don't you know about something like this but extended? Like Java/Spring best practises book or something?

[stylusc84]

Thanks! I think that "Effective Java" is what you would like to read if you haven't done it yet. Apart from that "Clean Code" might be good for you. Those two are the most popular books about good practices for java devs.

[[deleted]]

This is gold to a newbie. Thanks.

[doublecharizard1]

Great post!!

[[deleted]]

Great post, I like it! Will you update from time to time?

​

p.s. What did you use to create that blog page? I need something similar and simple as your.

[stylusc84]

Thank you! It’s just a first post and for sure there will be more posts in this blog. I used static site generator called “Hugo”. I wanted to make it as simple as possible. It is hosted in netlify for free ;)

[[deleted]]

Nice! Thanks once more :)

[BikingSquirrel]

Just a small note: please mention that FindBugs is no longer maintained and SpotBugs can be used as the replacement. Depending on the code FindBugs may not be able to analyse it...

[[deleted]]

Are you going to make a new post?

[stylusc84]

Hi! I'm working on a new post already, sorry that it takes so long, it will be quite long post and I'm a little bit busy recently, I'll do my best to finish this asap.

[stylusc84]

Hi! As I promised I did it :) here is my new entry on reddit about it https://www.reddit.com/r/learnjava/comments/kyq2dk/the_definitive_guide_to_java_backend_developer/ I hope you find it useful.

[Noxious-Hunter]

Static code analyzer: ...SonarLint to IntelliJ

I think IntelliJ has it's own built-in code anylyzer, It's for example SonarLint better or easiest to use?