Java Interview Questions for Experienced Developers: 10 Code Review Tasks

These ten Java snippets look like code from a normal pull request, and each one hides a bug that breaks production.

When I conducted Java interviews, code review tasks were the best way to start a real conversation. A candidate can memorize what a HashMap does. Applying it to strange code is a different skill, and it shows the level faster than 100 questions about theory.

The tasks go from Java core to Spring Boot. I built every snippet on Spring Boot 4.1.1 (Hibernate 7.4.5, Jackson 3.1.5) and Java 21, and checked the main bugs with unit tests that reproduce them.

How to run a code review interview

Share the code and ask one question: “What’s wrong with it?” Don’t tell the candidate how many problems there are. Watch which problem catches their eye first and how they rank them. Then ask the follow-up questions from each task.

Java core

Task 1. Remove a user from a list

public class UserCleanup { public static List<String> removeUser(List<String> users, String userToRemove) { for (String user : users) { if (user == userToRemove) { users.remove(user); } } return users; } } 

1. If the caller passes a list from List.of, remove throws UnsupportedOperationException. The method signature promises nothing about mutability.
2. Removing inside a for-each loop throws ConcurrentModificationException on the next iteration. Remove the second-to-last element and nothing throws: the loop just stops and never checks the last element.
3. The == operator compares references. It works in a unit test because string literals are interned, and fails for a name that came from a request or a database.
4. The method changes the caller’s list and returns it, so the return value hides a side effect.
5. A null list throws NullPointerException.

A version that leaves the input alone and compares values builds a new list:

public class UserCleanup { public static List<String> withoutUser(List<String> users, String userToRemove) { return users.stream() .filter(user -> !Objects.equals(user, userToRemove)) .toList(); } } 

Follow-up questions: What changes if the input is a LinkedList? What does removeIf do differently from removing in a loop?

Task 2. Seats as HashMap keys

public class Seat { private String row; private final Integer number; public Seat(String row, Integer number) { this.row = row; this.number = number; } public void moveTo(String row) { this.row = row; } @Override public boolean equals(Object o) { return o instanceof Seat other && row.equals(other.row) && number == other.number; } } 

1. equals without hashCode. Two equal seats get different identity hash codes, so a HashMap never finds a booking by a new Seat object, and a HashSet keeps both.
2. The number is an Integer compared with ==. Integer.valueOf caches values from -128 to 127, so seat 127 equals seat 127, and seat 128 doesn’t equal seat 128.
3. A seat with a null row throws NullPointerException from equals.
4. moveTo changes a field that takes part in equality. Add the missing hashCode and a moved seat gets lost inside a HashSet, because it sits in the bucket of its old hash.

A record gives equals and hashCode over immutable components:

public record Seat(String row, int number) { } 

Follow-up questions: Why does a record need no hand-written hashCode, and when would you still write one? What breaks if a subclass adds a field and overrides equals?

Task 3. Visit counter in a Spring service

@Service public class VisitCounter { private final Map<String, Integer> visits = new HashMap<>(); public void visit(String page) { Integer count = visits.get(page); visits.put(page, count == null ? 1 : count + 1); } public int visits(String page) { return visits.get(page); } } 

1. A Spring service is a singleton, so every request thread shares one HashMap. HashMap isn’t thread-safe, and concurrent puts can lose entries.
2. Get, then put, is check-then-act. Two threads read 5 and both write 6. Swap in a ConcurrentHashMap and the update still gets lost: a thread-safe map doesn’t make two calls atomic.
3. visits throws NullPointerException for a page nobody opened, because a null Integer is unboxed to int.
4. The page comes from the request. Every new query string adds a key that nothing evicts, so the map grows until the heap runs out.

Atomic per-key updates fix the race:

@Service public class VisitCounter { private final ConcurrentHashMap<String, LongAdder> visits = new ConcurrentHashMap<>(); public void visit(String page) { visits.computeIfAbsent(page, key -> new LongAdder()).increment(); } public long visits(String page) { LongAdder count = visits.get(page); return count == null ? 0 : count.sum(); } } 

The fix still counts per instance: three pods keep three counters. The keys still need a limit, for example only known routes.

Follow-up questions: Is a synchronized method enough? When is LongAdder better than AtomicLong?

Task 4. Report with a parallel stream

public class ReportService { public List<MemberSummary> summarize(List<Member> members) { List<MemberSummary> summaries = new ArrayList<>(); members.parallelStream().forEach(member -> { if (member.inactive()) { members.remove(member); } else { summaries.add(MemberSummary.from(member)); } }); return summaries; } } 

This method can throw four exception types. While every member is active and the list is short, it usually works, so it can live in production for a long time.

1. ConcurrentModificationException. The stream iterates members, and remove changes the same list. The spliterator checks the modification count after its traversal and throws.
2. ArrayIndexOutOfBoundsException, or summaries that silently go missing. Pool threads call add on one unsynchronized ArrayList at the same time.
3. UnsupportedOperationException when the caller passes List.of or any unmodifiable list.
4. NullPointerException for a null list.

The method also changes the caller’s list, and a failure leaves it half changed. Parallel forEach doesn’t keep the order, and parallelStream adds overhead with nothing measured to justify it.

public class ReportService { public List<MemberSummary> summarize(List<Member> members) { return members.stream() .filter(member -> !member.inactive()) .map(MemberSummary::from) .toList(); } } 

Follow-up questions: Which thread pool runs a parallel stream? When does parallelStream help?

Security

Task 5. Login check with JdbcTemplate

@Repository public class LoginRepository { private final JdbcTemplate jdbc; public LoginRepository(JdbcTemplate jdbc) { this.jdbc = jdbc; } public boolean canLogin(String username, String password) { String sql = String.format( "SELECT COUNT(*) FROM users WHERE username = '%s' AND password = '%s'", username, password); Integer count = jdbc.queryForObject(sql, Integer.class); return count > 0; } } 

Only 3 out of 10 candidates found the critical issues in this task during my interviews.

1. SQL injection. String.format doesn’t escape anything. The first line as a username logs in without a password. The second line as a password logs in as anyone:

admin' -- ' OR '1'='1 

Passwords are stored as plain text, because the query compares the raw input with the column. One database leak, and every password is out, including the ones people reuse on other sites.

No validation. A null username becomes the text “null” in the query. Blank input goes straight to the database too.

COUNT(*) greater than zero hides duplicates. Nothing in this code notices two rows for one username.

A common follow-up: would plain JDBC fix it? No. Statement with concatenation has the same hole. Parameters fix it, whether you use JdbcTemplate or JPA:

@Repository public class LoginRepository { private final JdbcTemplate jdbc; private final PasswordEncoder passwordEncoder; public LoginRepository(JdbcTemplate jdbc, PasswordEncoder passwordEncoder) { this.jdbc = jdbc; this.passwordEncoder = passwordEncoder; } public boolean canLogin(String username, String password) { List<String> hashes = jdbc.queryForList( "SELECT password_hash FROM users WHERE username = ?", String.class, username); return hashes.size() == 1 && passwordEncoder.matches(password, hashes.get(0)); } } 

The column now holds a BCrypt hash, and the encoder compares the input with it in Java. Declare a BCryptPasswordEncoder bean, because Spring Boot doesn’t create one.

Follow-up questions: Why should password hashing be slow? Where else can user input reach SQL (ORDER BY, LIKE patterns)?

Task 6. Token authenticator

@Slf4j @Service public class TokenAuthenticator { private final ApiClientRepository clients; public TokenAuthenticator(ApiClientRepository clients) { this.clients = clients; } public Optional<ApiClient> authenticate(String token) { log.info("Authenticating token {}", token); String[] parts = token.split("\\."); ObjectMapper mapper = new ObjectMapper(); Map<String, Object> header = mapper.readValue(decode(parts[0]), Map.class); Map<String, Object> claims = mapper.readValue(decode(parts[1]), Map.class); log.debug("Token signed with {}", header.get("alg")); return clients.findByLogin((String) claims.get("sub")); } private static byte[] decode(String part) { return Base64.getDecoder().decode(part); } } 

1. The signature is never checked. A token with a random signature passes, and so does a token with the header alg set to none. Anyone can write any subject.
2. No expiration check. A token stolen a year ago still works.
3. The full token goes to the INFO log, so anyone who reads logs can reuse it.
4. A new ObjectMapper on every call. The mapper is expensive to build and thread-safe to share. During a load test I once saw a flame graph where 70% of CPU time was spent creating object mappers.
5. Map plus casts. A numeric subject throws ClassCastException at runtime.
6. JWT parts are base64url. The basic decoder rejects the – and _ characters, so some valid tokens fail with IllegalArgumentException.
7. A string without dots throws ArrayIndexOutOfBoundsException, an unhandled exception instead of a clean 401.

Don’t parse tokens by hand. Spring Security’s resource server checks the signature and the expiration. It checks the issuer too once you set it below. The Boot 4 starter replaces the deprecated spring-boot-starter-oauth2-resource-server, and start.spring.io adds it when you pick OAuth2 Resource Server:

<!-- pom.xml --> <dependency> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-starter-security-oauth2-resource-server</artifactId> </dependency> 

// build.gradle.kts implementation("org.springframework.boot:spring-boot-starter-security-oauth2-resource-server") 

# application.properties spring.security.oauth2.resourceserver.jwt.issuer-uri=https://auth.example.com 

When code needs JSON, inject the JsonMapper that Spring Boot 4 configures for Jackson 3 instead of building a new one.

Follow-up questions: What does the alg header change? Why is a symmetric key risky when many services verify tokens?

Spring and Spring Data JPA

Task 7. Password change

@Service public class PasswordService { @Autowired private AppUserRepository users; @Autowired private AuditLogRepository auditLogs; public void updatePassword(Long userId, String oldPassword, String newPassword) { AppUser user = users.findById(userId).get(); saveAuditLog(userId, "PASSWORD_CHANGE"); user.setPassword(newPassword); users.save(user); } @Transactional(propagation = Propagation.REQUIRES_NEW) public void saveAuditLog(Long userId, String action) { auditLogs.save(new AuditLog(userId, action, LocalDateTime.now())); } } 

This task got the most reactions of all my interview posts on LinkedIn. The problems, from critical to minor:

1. Anyone can change anyone’s password. oldPassword is never checked, and the user id comes from the caller instead of the security context.
2. The new password is stored as plain text and never validated. A blank password passes.
3. The audit row is saved first and committed on its own. When the update fails (for example, a null password hits a NOT NULL column), the audit log says the password changed.
4. REQUIRES_NEW does nothing here. updatePassword calls saveAuditLog on this, not through the Spring proxy, so no new transaction starts. Called from another bean, the same method does get its own transaction.
5. updatePassword has no transaction. The read and the write run in two separate repository transactions.
6. Field injection hides dependencies from the constructor and makes the class harder to test.
7. findById(…).get() throws NoSuchElementException with the message “No value present”, without the id.
8. The action is a free string, and LocalDateTime.now() takes the server’s time zone.

A fixed version takes the user from the security context and checks the old password with PasswordEncoder before it stores a hash of the new one. The change and its audit row commit together in one transactional method. The bean gets its repositories through the constructor.

Follow-up questions: Which annotations work through the same proxy? Why can’t a private method be transactional?

Task 8. Wallet entity and service

@Data @Entity public class Wallet { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; private String owner; private double balance; @Enumerated(EnumType.ORDINAL) private WalletStatus status; private String createdAt; } 

public interface WalletRepository extends JpaRepository<Wallet, Long> { @Modifying @Query("update Wallet w set w.status = :status") int setStatusForAll(WalletStatus status); } 

@Slf4j @Service public class WalletService { private final WalletRepository wallets; public WalletService(WalletRepository wallets) { this.wallets = wallets; } public void rename(Long id, String owner) { Wallet wallet = wallets.findById(id).get(); wallet.setOwner(owner); } @Transactional public List<Wallet> freezeAll() { wallets.findAll().forEach(w -> log.info("Freezing wallet of {}", w.getOwner())); wallets.setStatusForAll(WalletStatus.FROZEN); return wallets.findAll(); } } 

1. rename does nothing. Outside a web request, findById runs in its own short transaction and returns a detached entity, so setOwner changes only an object in memory.
2. freezeAll returns stale data. The bulk update goes straight to the database and skips the persistence context. The second findAll returns the same managed wallets, still ACTIVE, while the table says FROZEN.
3. Lombok @data builds hashCode over every field, including the id. save sets the id on the same object, and a wallet put in a HashSet before saving can’t be found after.
4. ORDINAL stores the position of the constant: FROZEN is 1. Add a constant in the middle of the enum and old rows change meaning.
5. A double balance drifts: three deposits of 0.1 don’t add up to 0.3.
6. IDENTITY ids turn off JDBC insert batching in Hibernate, because Hibernate has to run each insert at once to learn its id. More on batching in my Spring Data JPA batch insert article.
7. createdAt is a String: no time zone, and text sorting breaks on the first different format.

Fixes for the entity: getters and setters instead of @data, EnumType.STRING, BigDecimal, Instant, and a sequence. rename needs @Transactional; dirty checking then saves the change without a save call. The bulk update needs a transaction and a cleared persistence context:

public interface WalletRepository extends JpaRepository<Wallet, Long> { @Transactional // a write needs a transaction, whoever calls it @Modifying(flushAutomatically = true, clearAutomatically = true) // fresh reads after the update @Query("update Wallet w set w.status = :status") int setStatusForAll(WalletStatus status); } 

Follow-up questions: What does dirty checking need to work? What does open-in-view hide in a case like rename?

Task 9. SMS after shipping

@Service public class ShippingNotifier { private final SmsGateway sms; private final NotificationRepository notifications; public ShippingNotifier(SmsGateway sms, NotificationRepository notifications) { this.sms = sms; this.notifications = notifications; } @Transactional public void onOrderShipped(OrderShipped event) { String messageId = sms.send(event.phone(), "Your order is on the way"); notifications.save(new Notification(event.orderId(), messageId)); } } 

A real case cost a company $2,000 in duplicate SMS messages: a Kafka consumer sent the SMS and then did one more write. This snippet has the same shape.

1. @Transactional doesn’t make the flow atomic. The SMS goes out first, then the insert fails, the transaction rolls back, and the customer already has the message.
2. No idempotency. Kafka delivers at least once, so the same event can arrive again after a timeout or a rebalance. The second delivery sends a second SMS and then fails on the unique order id, and a retrying consumer can repeat that.
3. The transaction holds a pooled database connection during the HTTP call to the SMS provider. A slow provider can drain the pool for the whole service.

A safer shape: save a pending notification with the order id as the idempotency key. Send the SMS after the commit, from an outbox table or an AFTER_COMMIT listener. Mark it sent, and only then acknowledge the Kafka offset.

Follow-up questions: What happens if the service crashes after the SMS but before it marks the notification sent? Where does the consumer acknowledge the message?

Task 10. A cleanup job with a green test

This one is close to a bug I shipped myself: the test for a scheduled @Modifying query was green, and in production the job silently did nothing.

@Component public class SessionCleanup { private final LoginSessionRepository sessions; public SessionCleanup(LoginSessionRepository sessions) { this.sessions = sessions; } @Scheduled(cron = "0 0 * * * *") public void removeExpired() { sessions.deleteExpired(Instant.now()); } } 

public interface LoginSessionRepository extends JpaRepository<LoginSession, L...