Chapitre 5: Débogage & Revue de Code
Ces exercices simulent les questions d'entrevue où l'on vous présente du code bugué ou mal écrit et on vous demande de trouver les problèmes et de les corriger. C'est un format très populaire chez les grandes entreprises (style Sonar/Codility).
Exercice 1 — Trouver et corriger 4 bugs
"Ce code a 4 bugs. Trouvez-les et corrigez-les."
Le code bugué
public class DiscountCalculator {
public double calculateDiscount(List<Product> products, String coupon) {
double total = 0;
for (int i = 0; i <= products.size(); i++) { // BUG 1
Product p = products.get(i);
if (p.getPrice() > 0) {
total =+ p.getPrice(); // BUG 2
}
}
double discount = 0;
if (coupon == "SAVE20") { // BUG 3
discount = total * 0.20;
}
return total - discount; // BUG 4: pas de null check
}
}
Le code corrigé
public class DiscountCalculator {
public double calculateDiscount(List<Product> products, String coupon) {
if (products == null || products.isEmpty()) return 0; // FIX 4
double total = 0;
for (int i = 0; i < products.size(); i++) { // FIX 1: < au lieu de <=
Product p = products.get(i);
if (p.getPrice() > 0) {
total += p.getPrice(); // FIX 2: += au lieu de =+
}
}
double discount = 0;
if ("SAVE20".equals(coupon)) { // FIX 3: .equals() au lieu de ==
discount = total * 0.20;
}
return total - discount;
}
}
Explication des bugs
| Bug | Problème | Conséquence | Correction |
|---|---|---|---|
| 1 | <= au lieu de < | ArrayIndexOutOfBoundsException | Changer en < |
| 2 | =+ au lieu de += | =+ assigne +valeur (positif), ne cumule pas | Changer en += |
| 3 | == compare les références | Deux String identiques peuvent avoir des références différentes | Utiliser .equals() |
| 4 | Pas de vérification null | NullPointerException si products est null | Ajouter un guard clause |
=+vs+=: L'un assigne, l'autre additionne. Très subtil visuellement.==vs.equals(): En Java,==compare les adresses mémoire, pas le contenu.- Off-by-one (
<=vs<) : L'erreur la plus classique en programmation.
"I found four bugs: off-by-one error in the loop, assignment instead of addition, reference comparison instead of .equals(), and missing null safety. These are classic Java pitfalls."
Exercice 2 — Revue de code complète
"Faites une revue de ce code. Qu'amélioreriez-vous ?"
Le code à examiner
public class DataProcessor {
public static ArrayList<String> process(ArrayList<String> data) {
ArrayList<String> result = new ArrayList<>();
for (int i = 0; i < data.size(); i++) {
String s = data.get(i);
if (s != null) {
if (s.length() > 0) {
if (!s.equals("SKIP")) {
result.add(s.toUpperCase());
}
}
}
}
return result;
}
}
Le code amélioré
public class DataProcessor {
public List<String> process(List<String> data) {
if (data == null) return List.of();
return data.stream()
.filter(Objects::nonNull)
.filter(s -> !s.isEmpty())
.filter(s -> !"SKIP".equals(s))
.map(String::toUpperCase)
.toList();
}
}
Tous les problèmes identifiés
| # | Problème | Correction | Principe |
|---|---|---|---|
| 1 | ArrayList dans la signature | Utiliser List (interface) | Programmer contre l'interface |
| 2 | Méthode static | Retirer static | Testabilité (mocking) |
| 3 | Pas de guard clause pour null | Ajouter if (data == null) return List.of() | Null safety |
| 4 | if imbriqués (3 niveaux) | Stream pipeline | Lisibilité |
| 5 | s.equals("SKIP") | "SKIP".equals(s) | Éviter NPE |
| 6 | s.length() > 0 | s.isEmpty() | Code idiomatique |
| 7 | ArrayList mutable retournée | toList() retourne une liste immuable | Immuabilité |
"I'd refactor to use streams for readability, program to interfaces, and add null safety. This is exactly the kind of code Sonar would flag — nested conditionals and mutable state."
Exercice 3 — Trouver la fuite mémoire
"Ce code cause une fuite mémoire. Trouvez-la."
Le code problématique
@Service
public class CacheService {
private final Map<String, Object> cache = new HashMap<>();
public void put(String key, Object value) {
cache.put(key, value);
}
public Object get(String key) {
return cache.get(key);
}
// Pas de méthode pour vider le cache !
// Le Map grandit indéfiniment → OutOfMemoryError
}
Le code corrigé
@Service
public class CacheService {
private final Map<String, Object> cache = new LinkedHashMap<>() {
@Override
protected boolean removeEldestEntry(Map.Entry<String, Object> eldest) {
return size() > 1000; // Limite à 1000 entrées
}
};
// Ou mieux, utiliser Caffeine/Redis
}
Encore mieux : utiliser Spring Cache
@Service
public class ProductService {
@Cacheable(value = "products", key = "#id")
public Product findById(Long id) {
return productRepository.findById(id).orElseThrow();
}
@CacheEvict(value = "products", key = "#id")
public void delete(Long id) {
productRepository.deleteById(id);
}
}
"A HashMap cache without eviction policy is a memory leak waiting to happen. I'd use Spring Cache with Caffeine or Redis, which handle TTL and max size automatically."
Exercice 4 — Trouver le problème de concurrence
"Ce code est thread-safe ? Trouvez le problème."
Le code problématique
@Service
public class CounterService {
private int count = 0;
public void increment() {
count++; // PAS thread-safe !
}
public int getCount() {
return count;
}
}
Problème : count++ n'est pas atomique. En réalité, c'est 3 opérations :
- Lire
count - Ajouter 1
- Écrire la nouvelle valeur
Si deux threads font count++ en même temps, on peut perdre un incrément.
Les solutions
// Solution 1 : AtomicInteger
@Service
public class CounterService {
private final AtomicInteger count = new AtomicInteger(0);
public void increment() {
count.incrementAndGet();
}
public int getCount() {
return count.get();
}
}
// Solution 2 : synchronized
@Service
public class CounterService {
private int count = 0;
public synchronized void increment() {
count++;
}
public synchronized int getCount() {
return count;
}
}
"count++ is not atomic — it's read-modify-write. I'd use AtomicInteger for lock-free thread safety, or synchronized if I need to protect a larger critical section."
Exercice 5 — Corriger une mauvaise gestion d'exceptions
"Qu'est-ce qui ne va pas avec cette gestion d'exceptions ?"
Le mauvais code
@Service
public class PaymentService {
public void processPayment(PaymentRequest req) {
try {
gateway.charge(req.getAmount());
orderService.updateStatus(req.getOrderId(), "PAID");
} catch (Exception e) {
// PROBLÈME 1 : catch trop large
// PROBLÈME 2 : avale l'exception silencieusement
// PROBLÈME 3 : log inutile
System.out.println("Error: " + e.getMessage());
}
}
}
Le code corrigé
@Service
@Slf4j
public class PaymentService {
@Transactional
public void processPayment(PaymentRequest req) {
try {
gateway.charge(req.getAmount());
} catch (PaymentDeclinedException e) {
log.warn("Payment declined for order {}: {}", req.getOrderId(), e.getMessage());
throw new BusinessException("Payment was declined", e);
} catch (GatewayTimeoutException e) {
log.error("Gateway timeout for order {}", req.getOrderId(), e);
throw new ServiceUnavailableException("Payment gateway is down", e);
}
orderService.updateStatus(req.getOrderId(), "PAID");
}
}
Problèmes identifiés
| # | Problème | Pourquoi c'est grave |
|---|---|---|
| 1 | catch (Exception e) | Attrape tout, y compris NullPointerException, OutOfMemoryError |
| 2 | Exception avalée | L'appelant ne sait jamais que le paiement a échoué |
| 3 | System.out.println | Pas de log structuré, pas de stack trace, perdu en production |
| 4 | Pas de rollback | Si le charge réussit mais le updateStatus échoue, données incohérentes |
"Never catch generic Exception. Catch specific exceptions, log with proper severity (warn vs error), re-throw as business exceptions, and use @Transactional for data consistency."
Exercice 6 — Identifier les code smells
"Listez tous les problèmes dans ce code."
Le code à analyser
public class Utils {
public static String x(String s) {
String r = "";
for (int i = s.length() - 1; i >= 0; i--) {
r = r + s.charAt(i);
}
return r;
}
public static boolean check(String email) {
if (email.contains("@") == true) {
if (email.contains(".") == true) {
return true;
} else {
return false;
}
} else {
return false;
}
}
public static int calc(int a, int b, String op) {
if (op == "+") return a + b;
if (op == "-") return a - b;
if (op == "*") return a * b;
if (op == "/") return a / b;
return 0;
}
}
Les problèmes
| Méthode | Problèmes |
|---|---|
x() | Nom incompréhensible, concaténation en boucle (O(n²)), pas de null check |
check() | == true redondant, if/else return true/false remplaçable par expression |
calc() | == au lieu de .equals(), division par zéro, retourne 0 silencieusement |
Le code corrigé
public class StringUtils {
public static String reverse(String input) {
if (input == null) return null;
return new StringBuilder(input).reverse().toString();
}
}
public class EmailValidator {
public static boolean isValid(String email) {
return email != null && email.contains("@") && email.contains(".");
}
}
public class Calculator {
public static int calculate(int a, int b, String operator) {
return switch (operator) {
case "+" -> a + b;
case "-" -> a - b;
case "*" -> a * b;
case "/" -> {
if (b == 0) throw new ArithmeticException("Division by zero");
yield a / b;
}
default -> throw new IllegalArgumentException("Unknown operator: " + operator);
};
}
}
Checklist de revue de code
Quand on vous demande de "reviewer" du code en entrevue, vérifiez systématiquement :
| Catégorie | Points à vérifier |
|---|---|
| Null safety | Vérifications null aux entrées ? Optional utilisé ? |
| Nommage | Noms explicites ? Conventions Java (camelCase) ? |
| Exceptions | Catch spécifique ? Pas d'exception avalée ? |
| Strings | .equals() pas == ? StringBuilder pour concaténation en boucle ? |
| Collections | Interfaces (List, pas ArrayList) ? Immuables quand possible ? |
| SOLID | Une seule responsabilité ? Dépend d'abstractions ? |
| Thread safety | Variables partagées protégées ? |
| Performance | Pas de N+1 ? Pas de boucle O(n²) cachée ? |
| Sécurité | Pas d'injection SQL ? Entrées validées ? |