-
Notifications
You must be signed in to change notification settings - Fork 135
SE 06 Clean Code
Part of the Software Engineering Principles series
Clean code is code that is easy for humans to read, understand, and modify. It is not just code that works — production systems are full of code that "works" but is impossible to maintain.
Robert C. Martin defines clean code as code that:
- Is focused — each function, class, and module does one thing well
- Has no surprises — it does what its name suggests
- Is easy to test
- Contains no duplication
- Reads like well-written prose
The measure of clean code is not the author's understanding but the reader's. Code is read far more often than it is written.
Names are the primary communication tool in code. A good name removes the need for a comment.
Name variables for what they represent, not what type they are or how they are used mechanically.
// Bad — what is d? What unit?
int d = 30;
// Good
int appointmentExpiryDays = 30;// Bad — Hungarian notation adds noise, not meaning
String strPatientName;
List<Bill> lstBills;
// Good
String patientName;
List<Bill> bills;Methods should be named as verbs or verb phrases that describe what they do. Boolean methods should read as questions.
// Bad
boolean check(Patient p);
void process(Bill b);
List<Item> get(Department d, Date from, Date to);
// Good
boolean isEligibleForDiscount(Patient patient);
void finalizeAndPostBill(Bill bill);
List<Item> findExpiredItemsInDepartment(Department dept, DateRange range);Class names should be nouns — the thing the class represents.
// Bad — describes a process, not a thing
class BillProcessing { }
class DataManager { }
// Good
class BillFinalizationService { }
class PatientRepository { }// Misleading — "list" implies ordering or indexing; it's actually a set
Map<String, Patient> patientList;
// Misleading — accountData with no indication of what kind
Object accountData;
// Misleading — very similar names for different purposes
String patientAddress;
String patientAddrs;// Bad — unpronounceable, unsearchable
Date genymdhms;
int cntPts;
// Good
Date generatedTimestamp;
int patientCount;A function should do one thing. If you need the word "and" to describe what it does, it is doing too much.
// Bad — does three things
public void processAdmission(Patient patient, Ward ward) {
// validate patient
if (patient.getId() == null) throw new RuntimeException("No ID");
// allocate bed
Bed bed = bedRepository.findAvailable(ward);
bed.allocate(patient);
// create billing account
billingService.openAccount(patient, AdmissionType.INPATIENT);
}
// Good — orchestrates clearly named single-purpose methods
public Admission admitPatient(Patient patient, Ward ward) {
validatePatientForAdmission(patient);
Bed bed = allocateBed(ward, patient);
BillingAccount account = openBillingAccount(patient);
return new Admission(patient, bed, account);
}Fewer arguments is better. More than three arguments is a signal to introduce a parameter object.
// Bad — hard to call correctly, easy to swap arguments
public Bill createBill(Patient patient, Department dept, Staff staff,
BillType type, PaymentMethod method, double amount) { }
// Good
public Bill createBill(BillCreationRequest request) { }
// BillCreationRequest groups the parameters and can be validated independentlyA function should either do something (command) or answer something (query) — not both.
// Bad — does this return the saved entity or a success flag? Does it have side effects?
boolean savePatient(Patient patient);
// Good — separate concerns
void savePatient(Patient patient); // command: has side effect, returns nothing meaningful
boolean isPatientDuplicate(Patient patient); // query: answers a question, no side effectsThe best comment is a good name that makes the comment unnecessary.
// Bad — comment just repeats the code
// Increment patient count by one
patientCount++;
// Bad — explains WHAT, not WHY (the name should explain what)
// Get all active bills
List<Bill> getActiveBills() { }
// Bad — commented-out code
// List<Bill> bills = getBillsOld();
List<Bill> bills = getBills();// Good — explains WHY a non-obvious choice was made
// Intentional: we truncate to seconds because the legacy reporting system
// does not preserve milliseconds and equality checks would always fail.
LocalDateTime truncated = timestamp.truncatedTo(ChronoUnit.SECONDS);
// Good — explains a domain constraint the code cannot express
// Per MOH regulation 2019/47, lab results must be retained for 10 years.
private static final int LAB_RESULT_RETENTION_YEARS = 10;
// Good — warns of a known issue
// WARNING: EclipseLink fetches this collection eagerly despite LAZY annotation
// when accessed within the same transaction. See issue #1234.
@OneToMany(fetch = FetchType.LAZY)
private List<BillItem> items;Formatting is communication. Consistent formatting reduces the cognitive effort of reading code.
Group related lines together and separate unrelated ones with a blank line.
// Bad — no breathing room, everything merged
public Bill createBill(Patient patient, double amount) {
validatePatient(patient);
validateAmount(amount);
Bill bill = new Bill();
bill.setPatient(patient);
bill.setNetValue(amount);
bill.setCreatedDateTime(LocalDateTime.now());
bill.setStatus(BillStatus.DRAFT);
billRepository.save(bill);
auditService.log("BILL_CREATED", bill.getId());
return bill;
}
// Good — blank lines separate distinct logical steps
public Bill createBill(Patient patient, double amount) {
validatePatient(patient);
validateAmount(amount);
Bill bill = new Bill();
bill.setPatient(patient);
bill.setNetValue(amount);
bill.setCreatedDateTime(LocalDateTime.now());
bill.setStatus(BillStatus.DRAFT);
billRepository.save(bill);
auditService.log("BILL_CREATED", bill.getId());
return bill;
}Keep lines under ~120 characters. Long lines are hard to read and require horizontal scrolling.
Whatever conventions are chosen for a project — indentation size, brace style, import ordering — apply them uniformly. Inconsistency forces the reader to adapt constantly.
// Bad — callers forget to check the return code
int result = savePatient(patient);
if (result == -1) { /* sometimes this check is forgotten */ }
// Good — exceptions cannot be silently ignored
void savePatient(Patient patient) throws PatientValidationException { }// Bad — catches too broadly, hides the real cause
try {
processPayment(bill);
} catch (Exception e) {
log("Error");
}
// Good — specific, meaningful handling
try {
processPayment(bill);
} catch (InsufficientFundsException e) {
notifyPatient(e.getPatient());
} catch (PaymentGatewayException e) {
scheduleRetry(bill);
}Returning null is a common source of NullPointerExceptions. Return Optional<T> or an empty collection instead.
// Bad — callers must remember to null-check every time
public Patient findPatient(String id) {
Patient p = repository.find(id);
return p; // may be null
}
// Good — makes absence explicit
public Optional<Patient> findPatient(String id) {
return Optional.ofNullable(repository.find(id));
}Leave the code cleaner than you found it.
You do not need to refactor an entire module every time you touch it. But when you open a file to fix a bug or add a feature, improve any small thing you notice: rename a confusing variable, extract a long method, remove a stale comment. Cumulatively, this discipline prevents code from decaying.
A code smell is a symptom that suggests a deeper problem. Common smells to recognise:
| Smell | Description | Typical remedy |
|---|---|---|
| Long method | Method too big to comprehend | Extract method |
| Large class | Class has too many responsibilities | Split the class |
| Long parameter list | Method takes too many arguments | Introduce parameter object |
| Duplicate code | Same logic in multiple places | Extract to shared method/class |
| Dead code | Unreachable or unused code | Delete it |
| Magic numbers | Unexplained numeric literals | Extract to named constant |
| Feature envy | Method uses another class's data more than its own | Move method |
| Inappropriate intimacy | Class knows too much about another's internals | Encapsulate |
Previous: SE-05: Design Patterns
Next: SE-07: Software Architecture