OK i need to change your code. SORRY

Ok. In this video I will review and improve a bunch of code. The aim is to teach you how to write better code.

Don't Forget to
===========================================

🙊 Here are the goods for all my videos video 🙊

► Recommended Books
===========================================

► Computer and Monitor
===========================================

► Camera Gear
=============================================

► IDE & Tools I use for coding 💻 🎒
===========================================
- ITerm
- VsCode
- GoLand
- IntelliJ Ultimate
- Sublime

P.S
===========================================

❤️ Thanks for watching

[mohammedsfr]

We want more of these series . It's very useful

[mateusloubach]

One of my main issues when I first started learning code, was not being able to understand the logic behind the code. And everyone teaching back then only focused on making sure we memorized the style of the code instead of the logic behind it. - THIS for me, is one of the BEST part of your videos. Since you explain exactly what that code does and then, when needed, fix it, explaining exactly what the fix does.

If there was a dedicated series on this, it would be amazing!!

fantastic work. love what you do.

[salih.k]

Just more of this. This was literally amazing.
I enjoyed every single second of it.

[aykborstelmann]

Actually regarding the comparator you wrote. You could just do
-> ...))

Then this would even be smaller and neater.

[IvanRandomDude]

19:25 Isn't REST standard to return 201 Created and include newly created object in the response body and URL in location header?

[michelchaghoury]

@AmigosCode, i really love the codereview, please can you make in the future some e2e tensting tutorials, as well as CI/CD, and Cloud (AWS or GCP) pleaseee, and keep going

[tinnick]

I’m not a Java programmer so please take this comment lightly.

I find that at 19:50, you refactored to not return anything but I think this differs depending on the context.

If the frontend needs to be aware of multiple bookings, it should return that saved booking id instead of void.

This is so that the frontend can be aware of the saved booking id to then maybe later in the same form send a patch request to update the booking.

[medabotsss]

Disagree with listAll
The class is called booking service and it only gets bookings.
Calling it getAll is better because it is less verbose and you can still figure out the meaning from the class name.

Same thing with delete, find, create

[edgelamer]

This is amazing. More of this! Watching a senior dev correcting lower tier devs is so nice and a valuable lesson for us all.

[dcascato]

Wow, I never learned so much in one video. I think the value here is you are tackling it from a professional perspective, which some of us newbies lack. Tutorials many time go around teaching things like syntax and that kind of stuff, but I believe learning best practices is really valuable. Just loved it

[oliwer]

Fair cleanup, but there are some points here that caught me off guard.

7:40 - I'm curious as to why it's best practice to only import the used types and not wildcard said package. The only "issue" there is, is that it clutters the local namespace and in reality could ONLY cause you to receive a compile-time error if there were to be conflicting type names, which can be resolved on the spot. Other than the mentioned "issue", there is no obvious reason not to do this if you're certain of the type namings. You also decrease the amount of unnecessary lines of code.

9:50 - 'listAll', 'all' or 'findAll' would be fine considering the fact that it's in a type of 'BookingService'. The type of listing is already made clear in the name so IMO seems very exhaustive and explicit.

10:36 - same point for this as the one mentioned above. 'BookingService' already indicate that it handles bookings and therefore, 'deleteByCode' references the code of booking(s).

29:29 - why would one NEVER reference the 'Collection' type? It's useful in many cases, and quite frankly required a lot.

Feel free to prove me wrong if there are reasonable doubt and arguments to the points mentioned. As a backend programmer myself, I would be glad to receive said thoughts as we can never stop learning. Other than that; fun video, Amigos!

[adamk]

20:30
honestly calling findAll on a repository and doing filtering in code is a huge code smell.
you're loading an entire DB table into memory just to filter out one single entry.
best case: it's just bad code, worst case: your operating cost explodes because of the memory consumption, also latency.
the filter itself should be contained within a specialized DB query to the likes of (pseudo-SQL:) "WHERE startDate before now AND confirmed == true" then "ORDER BY start date ASC" and "LIMIT 1", . the repository method returns an Optional, since you're actually only interesented in one entry (which could not exist, e.g. if you don't have any bookins yet, therefore Optional).
BOOM, your code just became extremely trivial.
also don't forget DB indexes on startDate and confirmed DB fields fro proper DB query performance ;)

[aadlani]

You are the best Trainer on Java so far in my experience.
Keep rocking. Big Fan

[Qrzychu]

Well, I must say that modern Java looks quite neat. Greeting from dotnet :)

[returncode]

The code review/refactor series is great, keep it 👍

[chandrakanthpadi]

I think no YouTuber had done this. This code reviews is so good to see. Please continue to.

[krammer]

This is awesome! I would suggest writing the test first and then re-factoring.

[luizfernandonoschang]

Awesome video man! I'm an experienced Java developer and I'm planning to take one of your online courses, because I'm learning a lot with you.

Now, I want to argue with you in one thing. I think that the BookingService should actually return a BookingDTO upon insertion. This is because, when you send a 200 CREATED status, usually you send back a link to the newly created resource, so the client can actually send a GET request afterwards.

In fact, mostly of the time the client will insert the new resource and do something with it afterwards(adding to a list on the UI, for example). So, sending the created entity avoids having to do a second request.

If returning the whole Entity/DTO may cause a overhead, then at least the generated ID should be returned, so you can build the link for the client. Well, this is my opinion.

Congratulations again for the video!

[froshiga]

WOW, I started as a junior java dev about a year ago and I learned more from these videos than most of my time at work!

[yogeshchaudhary]

Man I'm just loving such kind of videos. This teaches you so much practical stuff.