Skip to main content
18 votes
Accepted

Unit testing Video Service class

Welcome to code review and thanks for sharing your code. When writing UnitTests keep in mind that they are documentation. Test code has less restrictions for identifier names. This means that method ...
Timothy Truckle's user avatar
11 votes

Using static final Strings for SQL Query in Spring Boot

Sorry to present the antithesis to an existing answer once more: if the string is neither reusable nor public, a constant makes no sense in my book. (And yes, this question is opinion-based ;-)) I ...
mtj's user avatar
  • 4,822
11 votes
Accepted

How can I rewrite the below parser code in Java

First off, it would've been nice to see the entire method, or preferrably even the entire class. Having less context makes reviewing harder. That said, I feel there was enough for a review, so here ...
Sara J's user avatar
  • 3,471
9 votes

Spring Boot RESTful service

Should be changed: Request mappings are not that RESTful due to the verbs: POST /api/cars/add-car => POST /api/cars ...
Lyubomyr Shaydariv's user avatar
8 votes

Unit testing Video Service class

The idea of the test class is ok: you want to test the service, mock the repository and you check on the outcome. In the test, it would be nice to use the AAA (arrange/act/assert) pattern. In my ...
Manuel's user avatar
  • 311
7 votes
Accepted

Stack Overflow clone

General Feedback the package 'StackCanary' can follow Java standard naming. Ref: https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html You do not need to type Tag on ...
gtiwari333's user avatar
6 votes
Accepted

SMS sending through Clickatell

1 Use Logger, do not print all your app info, in production it will mess up server logs. You can use different log level (debug, info). For example use info level in prod, and debug level in dev ...
yvoytovych's user avatar
6 votes
Accepted

Is my implementation of a simple CRUD service with Spring WebFlux correct?

This is not necessarily an in-depth review, but I see one issue and one improvement: The CRUD method for delete in your service should combine operators Currently ...
simonbasle's user avatar
6 votes
Accepted

Rest Api with generic crud operations

Your code is good, but some improvements could be made. Here are my suggestions: Use lowercase for first letter of variable. Example: BrandService BrandService; ...
i.bondarenko's user avatar
6 votes

Java 8 use of optional

Optionals do not improve your code. You're using if statements and null checks just like they are intended to be used. There is no need to improve anything there. Edit: If-statements and null-checks ...
TorbenPutkonen's user avatar
6 votes

Spring boot application web service

fizzBuzzOutputsRepo is an in-memory map and there is no data persistence. So if the web server crashes, then there will be data loss. Even if data persistence is ...
Jia Ni's user avatar
  • 306
6 votes
Accepted

The best approach to validate fields

From an object-oriented point of view there is a better way you can handle this scenario. Feature Envy Currently the code snipped has a Code Smell that is called Feature Envy: The whole point of ...
Roman's user avatar
  • 2,843
6 votes

Are these Unit tests OK or am I implementing an antipattern or not following best practices

Some Concepts I'm not much experienced in the framework used but Unit Testing, Test Driven Development and Behavioral Driven Development are concepts that need to be understood well and then ...
Anit Shrestha's user avatar
6 votes
Accepted

Using java streams to handle incoming API requests

A "stream" is like a list or a queue; what you're doing is intelligible, but kind of silly because there's only ever one item. Judging from the alternative you post in the comments, it looks ...
ShapeOfMatter's user avatar
5 votes

Copying notes takes a very long time

You said that the method addAttachmentForCopy() is taking a lot of time. This method is doing some DAO operations, but also a few file reading operations. Are you ...
antonro's user avatar
  • 302
5 votes

Spring service for playing Rock Paper Scissors

SOLID Principles Single responsibility principle A class should have one, and only one, reason to change. This class should take a PlayerRequest and update the state of the corresponding Players. ...
mihh_don's user avatar
  • 101
5 votes

Spring boot application web service

From API design perspective let me pitch in Have a base path and then have the resource name in the corresponding request mapping. Base path /api version /v1 request mapping /fizzBuzz Add HTTP ...
Nagarajan's user avatar
5 votes

Spring boot application web service

PUT vs. POST @PutMapping("/fizzbuzz") public void createFizzBuzzItem(...) PUT and POST ...
Roman's user avatar
  • 2,843
5 votes

User Id generator

Code review UserIdFactoryImpl Why is this called Impl if it does not extend an interface? Why isn't it ...
Maarten Bodewes's user avatar
4 votes
Accepted

Spring Data Redis - mapping object relationships?

There's nothing really wrong with your modelling approach. If you feel confident with the modelling you are good to go. Data modelling in Redis depends on the functional and non-function requirements ...
Christian Rühl's user avatar
4 votes

Preventing XSS attacks in a Spring MVC application controller

You can use OWASP's ESAPI jar json = ESAPI.encoder().canonicalize(json); json = ESAPI.encoder().encodeForHTML(json); Dependency : ...
vaquar khan's user avatar
4 votes

Java 8 use of optional

For first method: ...
Ionut Bilica's user avatar
4 votes

Unit testing Video Service class

Some notes: If you're using mocks you are pretty much by definition doing integration rather than unit testing, because mocks enable you to test how a piece of code integrates with something else ...
l0b0's user avatar
  • 8,917
4 votes

Using static final Strings for SQL Query in Spring Boot

Not many valid reasons, why would ever isRecordExists2 be better method. It is almost always good to extract reusable strings into constants and split code into ...
K.H.'s user avatar
  • 2,668
4 votes
Accepted

Return a 404 when a resource is not found in Spring Boot

Your URL path isn't very RESTlike. You have built the URL to describe an operation when a more common approach is to build the URL to describe a resource. So instead of ...
TorbenPutkonen's user avatar
4 votes
Accepted

Get all data from an external and paginated Swagger API using Java / Feign / Spring

I see these lines in your code : List<Item> items = restPage.getContent(); List<Item> result = new ArrayList<Item>(); result.addAll(items); You ...
dariosicily's user avatar
  • 3,961
4 votes
Accepted

Spring Boot API - Avoiding Concurrency issues

Welcome to Code Review, because Spring demands to the user the responsibility of avoiding concurrency issues better always check if the possibility of concurrent issues exists. About your ...
dariosicily's user avatar
  • 3,961
3 votes

Transaction in spring boot

You can use Spring data CrudRepository which does a lot of things for you out of the box. Just create an interface and "map" it to the object you want to persist in the database, by writing the class ...
Urosh T.'s user avatar
  • 131
3 votes

Transaction in spring boot

Did you try to use google for this? You have multiple way to do this :P For example: @Transactional annotation on method. Or using XML file to wrap methods with ...
KirkoR's user avatar
  • 133

Only top scored, non community-wiki answers of a minimum length are eligible