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, Long> {
    @Modifying
    @Query("delete from LoginSession s where s.expiresAt < :now")
    int deleteExpired(Instant now);
}

@DataJpaTest
@Import(SessionCleanup.class)
class SessionCleanupTest {
    @Autowired
    SessionCleanup cleanup;
    @Autowired
    LoginSessionRepository sessions;
    @Test
    void removesExpiredSessions() {
        sessions.save(new LoginSession(Instant.now().minusSeconds(60)));
        cleanup.removeExpired();
        assertThat(sessions.count()).isZero();
    }
}

The test is green. In production the job throws InvalidDataAccessApiUsageException caused by TransactionRequiredException on every run, and the scheduler only logs it.

1. Spring Data JPA applies no transaction to your own query methods. A @Modifying query needs one, and the scheduler calls removeExpired without it.
2. @DataJpaTest wraps each test in a transaction. The test borrows that transaction, and production has none.

The fix puts the transaction on the write:

public interface LoginSessionRepository extends JpaRepository<LoginSession, Long> {
    @Transactional // the scheduler calls it without a transaction
    @Modifying
    @Query("delete from LoginSession s where s.expiresAt < :now")
    int deleteExpired(Instant now);
}

The bigger fix is in the tests. Run integration tests with @SpringBootTest and without @Transactional on test classes. An ArchUnit rule can fail the build when someone adds @DataJpaTest or @Transactional to an integration test again.

Follow-up questions: Which repository methods are transactional by default? How else can a test transaction hide a bug?

Spring Boot good practices to ask about

Tasks 5, 7, 8 and 9 break at least one of these, and a candidate can explain them without any code.

1. Constructor injection. Required dependencies are final, and a unit test can build the class without Spring.
2. HTTP stays in the controller layer. Services throw domain exceptions such as UserNotFoundException, and a @RestControllerAdvice maps them to responses.
3. Turn on Problem Details for errors. It is off by default:

# application.properties
spring.mvc.problemdetails.enabled=true

Return DTOs, not entities. An entity leaks internal fields such as a password hash, and lazy associations fail after the transaction closes.

Turn off open-in-view. Spring Boot 4.1 still turns it on by default and only logs a warning. It keeps the database session open while the response renders and hides lazy loading problems:

# application.properties
spring.jpa.open-in-view=false

# application.properties
spring.jpa.hibernate.ddl-auto=validate

Validate input at the edge with Bean Validation on request DTOs, then trust the types inside.

Tokens and secrets never go in the URL. Query parameters end up in access logs and proxy logs.

Every write that a client or a broker can retry needs an idempotency key.

Which of these tasks would you give a candidate first, and which bug did you miss on the first read?

At the end

If you like this post, follow me on LinkedIn for new code review tasks. More on entities and transactions: Spring Data JPA guide.

Found this useful? Follow me on social media to stay updated.

• Telegram
• LinkedIn