Bad Code Smells: The Silent Killers of Swift Elegance
Understanding code smells is essential for maintaining clean, efficient, and manageable Swift codebases. This tech blog explains various code smells that should be avoided by us as Swift developers to keep its elegance.
1. Bloaters
a) Long Method
Long methods handle multiple tasks and reduce readability.
Example:
func processOrder(order: Order) {
validate(order)
calculatePrice(order)
applyDiscount(order)
generateInvoice(order)
sendConfirmation(order)
}
Solution: Refactor into smaller methods.
func processOrder(order: Order) {
validate(order)
finalizeOrder(order)
}
func finalizeOrder(order: Order) {
calculatePrice(order)
applyDiscount(order)
generateInvoice(order)
sendConfirmation(order)
}
b) Large Class
A class with too many responsibilities.
Solution: Break into smaller classes with clear responsibilities.
c) Primitive Obsession
Excessive use of primitives instead of meaningful objects.
Example:
let userName: String
let userEmail: String
let userAddress: String
Solution: Create a dedicated object.
struct UserProfile {
let name: String
let email: String
let address: String
}
d) Data Clumps/Long Parameter List
Frequent use of grouped variables or methods with many parameters.
Solution: Encapsulate them into classes or structs.
struct UserDetails {
let name: String
let age: Int
let email: String
}
func createUser(details: UserDetails) {}
2. Object-Orientation Abusers
a) Switch Statements
Overuse of conditional logic.
Example:
enum PaymentType {
case creditCard
case paypal
case applePay
}
func processPayment(type: PaymentType, amount: Double) {
switch type {
case .creditCard:
print("Processing credit card payment of \(amount)")
case .paypal:
print("Processing PayPal payment of \(amount)")
case .applePay:
print("Processing Apple Pay payment of \(amount)")
}
}
Solution: Employ polymorphism or strategy pattern.
protocol PaymentStrategy {
func pay(amount: Double)
}
class CreditCardPayment: PaymentStrategy {
func pay(amount: Double) {
print("Processing credit card payment of \(amount)")
}
}
class PayPalPayment: PaymentStrategy {
func pay(amount: Double) {
print("Processing PayPal payment of \(amount)")
}
}
class ApplePayPayment: PaymentStrategy {
func pay(amount: Double) {
print("Processing Apple Pay payment of \(amount)")
}
}
// Usage
func processPayment(strategy: PaymentStrategy, amount: Double) {
strategy.pay(amount: amount)
}
// Example Usage
let paymentStrategy = PayPalPayment()
processPayment(strategy: paymentStrategy, amount: 50.0)
Using the strategy pattern eliminates conditional complexity, enhances flexibility, and adheres to the Open/Closed principle.
b) Temporary Field
Fields used only under certain conditions, indicating poor design.
Example:
class UserSession {
var userName: String
var sessionToken: String?
var expiryDate: Date?
init(userName: String) {
self.userName = userName
}
func login(token: String, expiry: Date) {
self.sessionToken = token
self.expiryDate = expiry
}
func logout() {
self.sessionToken = nil
self.expiryDate = nil
}
}
In this example, sessionToken and expiryDate are temporary fields, only relevant during an active session.
Solution: Extract fields into separate classes or contexts.
struct Session {
let token: String
let expiryDate: Date
}
class User {
let userName: String
var session: Session?
init(userName: String) {
self.userName = userName
}
func login(session: Session) {
self.session = session
}
func logout() {
self.session = nil
}
}
By extracting temporary fields into their own context (Session), we improve class clarity, ensure single responsibility, and make our code easier to maintain.
c) Refused Bequest
Subclasses that don’t use all inherited methods.
Example:
class Bird {
func fly() {
print("Flying")
}
func swim() {
print("Swimming")
}
}
class Sparrow: Bird {
// Sparrows can't swim; the swim method is irrelevant
}
let sparrow = Sparrow()
sparrow.fly() // Fine
sparrow.swim() // Irrelevant method inherited unnecessarily
In this scenario, Sparrow inherits the method swim() which it doesn’t need, resulting in refused bequest.
Solution: Use composition instead of inheritance.
protocol Flyable {
func fly()
}
extension Flyable {
func fly() {
print("fly")
}
}
protocol Swimmable {
func swim()
}
extension Swimmable {
func swim() {
print("swim")
}
}
class Sparrow: Flyable {
}
class Duck: Flyable & Swimmable {
}
// Usage
let sparrow = Sparrow()
sparrow.fly()
let duck = Duck()
duck.fly()
duck.swim()
Using composition (via protocols and separate behavior classes) avoids unnecessary method inheritance and leads to clearer, more flexible, and maintainable designs.
d) Alternative Classes with Different Interfaces
Similar functionality with inconsistent interfaces.
Example:
class JSONLogger {
func logJSON(message: String) {
print("{\"log\": \"\(message)\"}")
}
}
class XMLLogger {
func writeXMLLog(message: String) {
print("<log>\(message)</log>")
}
}
// Usage
let jsonLogger = JSONLogger()
jsonLogger.logJSON(message: "System error")
let xmlLogger = XMLLogger()
xmlLogger.writeXMLLog(message: "System error")
In this example, both classes perform similar logging tasks but with inconsistent interfaces.
Solution: Refactor into common interfaces or inheritance.
protocol Logger {
func log(message: String)
}
class JSONLogger: Logger {
func log(message: String) {
print("{\"log\": \"\(message)\"}")
}
}
class XMLLogger: Logger {
func log(message: String) {
print("<log>\(message)</log>")
}
}
// Usage
let loggers: [Logger] = [JSONLogger(), XMLLogger()]
for logger in loggers {
logger.log(message: "System error")
}
By refactoring into a shared interface (Logger protocol), consistency improves, enhancing flexibility, readability, and maintainability.
3. Change Preventers
a) Divergent Change
One class changing for multiple reasons.
Example:
class UserManager {
func authenticateUser(username: String, password: String) {
// authentication logic
}
func updateProfile(userID: Int, newProfileData: [String: Any]) {
// update user profile logic
}
func logActivity(activity: String) {
// logging user activities
}
}
In this example, UserManager changes for unrelated reasons—authentication logic, profile management, and logging.
Solution: Split into classes based on responsibilities.
class AuthenticationService {
func authenticate(username: String, password: String) {
// authentication logic
}
}
class UserProfileService {
func updateProfile(userID: Int, newProfileData: [String: Any]) {
// profile update logic
}
}
class ActivityLogger {
func log(activity: String) {
// logging logic
}
}
// Usage
let authService = AuthenticationService()
authService.authenticate(username: "user", password: "password123")
let profileService = UserProfileService()
profileService.updateProfile(userID: 42, newProfileData: ["name": "Jane"])
let logger = ActivityLogger()
logger.log(activity: "User logged in")
By clearly separating these responsibilities, the classes become cohesive, easier to maintain, and less prone to unintended side effects during changes.
b) Shotgun Surgery
Single changes affecting multiple classes.
Example:
class UserProfile {
func updateName(userID: Int, newName: String) {
// update logic
}
}
class UserSettings {
func updateName(userID: Int, newName: String) {
// update logic
}
}
class UserNotifications {
func updateName(userID: Int, newName: String) {
// update logic
}
}
Here, updating the user’s name requires modifying multiple unrelated classes, causing scattered and duplicated changes.
Solution: Consolidate related behavior into fewer classes.
class User {
func updateName(userID: Int, newName: String) {
updateProfileName(userID: userID, newName: newName)
updateSettingsName(userID: userID, newName: newName)
updateNotificationsName(userID: userID, newName: newName)
}
private func updateProfileName(userID: Int, newName: String) {
// profile name update logic
}
private func updateSettingsName(userID: Int, newName: String) {
// settings name update logic
}
private func updateNotificationsName(userID: Int, newName: String) {
// notifications name update logic
}
}
// Usage
let user = User()
user.updateName(userID: 42, newName: "Alice")
By centralizing the related behavior into a single class (User), changes are streamlined, reducing the complexity and risk associated with scattered updates.
c) Parallel Inheritance Hierarchies
Multiple related hierarchies changing together.
Example:
// UI Elements hierarchy
class Button {}
class RoundedButton: Button {}
class SquareButton: Button {}
// Style hierarchy
class ButtonStyle {}
class RoundedButtonStyle: ButtonStyle {}
class SquareButtonStyle: ButtonStyle {}
In this example, every time a new type of Button is added, a corresponding style subclass must be created, leading to tightly coupled parallel hierarchies.
Solution: Merge hierarchies or use composition.
// Button styles
protocol Style {
func applyStyle()
}
class RoundedStyle: Style {
func applyStyle() {
print("Applying rounded style")
}
}
class SquareStyle: Style {
func applyStyle() {
print("Applying square style")
}
}
// Button with composition
class Button {
private let style: Style
init(style: Style) {
self.style = style
}
func render() {
style.applyStyle()
print("Rendering button")
}
}
// Usage
let roundedButton = Button(style: RoundedStyle())
roundedButton.render()
let squareButton = Button(style: SquareStyle())
squareButton.render()
Using composition to combine different styles eliminates parallel inheritance, reduces tight coupling, and significantly improves flexibility and maintainability.
4. Dispensables
a) Comments
Excessive or outdated comments indicating poor readability.
Example:
// User class stores user information
class User {
// User's name
var name: String
// Initialize a new User
init(name: String) {
// Set user's name
self.name = name
}
// Function to update user's name
func updateName(newName: String) {
// Assign new name to user
self.name = newName
}
}
These comments don’t add clarity, as the code itself is already clear and readable.
Solution: Improve code readability and remove redundant comments.
class User {
var name: String
init(name: String) {
self.name = name
}
func updateName(to newName: String) {
name = newName
}
}
By writing self-explanatory, clearly named methods and variables, we reduce the need for excessive comments, resulting in cleaner and more maintainable code.
b) Duplicate Code
Repeated code segments.
Example:
class ProfileViewController {
func displayFormattedDate(date: Date) -> String {
let formatter = DateFormatter()
formatter.dateStyle = .medium
formatter.timeStyle = .short
return formatter.string(from: date)
}
}
class NotificationViewController {
func showFormattedDate(date: Date) -> String {
let formatter = DateFormatter()
formatter.dateStyle = .medium
formatter.timeStyle = .short
return formatter.string(from: date)
}
}
Here, both classes contain identical date formatting logic, leading to repetitive and duplicated code.
Solution: Extract common functionality.
struct DateFormatterHelper {
static let shared = DateFormatterHelper()
private let formatter: DateFormatter
private init() {
formatter = DateFormatter()
formatter.dateStyle = .medium
formatter.timeStyle = .short
}
func formattedDate(_ date: Date) -> String {
formatter.string(from: date)
}
}
class ProfileViewController {
func displayFormattedDate(date: Date) -> String {
DateFormatterHelper.shared.formattedDate(date)
}
}
class NotificationViewController {
func showFormattedDate(date: Date) -> String {
DateFormatterHelper.shared.formattedDate(date)
}
}
Extracting repetitive code into reusable components (like DateFormatterHelper) ensures a single source of truth, simplifies maintenance, and prevents inconsistencies.
c) Dead Code
Unused code cluttering the codebase.
Example:
class PaymentProcessor {
func processPayment(amount: Double) {
print("Processing payment of \(amount)")
}
// Unused function left behind
func oldPaymentMethod(amount: Double) {
print("Old payment method for \(amount)")
}
}
let processor = PaymentProcessor()
processor.processPayment(amount: 100.0)
// oldPaymentMethod is never called
In this example, oldPaymentMethod is dead code, adding unnecessary clutter to the class.
Solution: Regularly remove unnecessary code.
class PaymentProcessor {
func processPayment(amount: Double) {
print("Processing payment of \(amount)")
}
}
let processor = PaymentProcessor()
processor.processPayment(amount: 100.0)
Regularly removing dead code streamlines our codebase, reduces complexity, and makes future maintenance tasks simpler and more efficient.
d) Speculative Generality
Code written for hypothetical future requirements.
Example:
protocol DataSource {
func fetchData() -> [String]
}
class LocalDataSource: DataSource {
func fetchData() -> [String] {
return ["Local data"]
}
}
// Hypothetical class created for future remote source, currently unused
class RemoteDataSource: DataSource {
func fetchData() -> [String] {
// Implementation to be added in future
return []
}
}
class DataManager {
let dataSource: DataSource = LocalDataSource()
func loadData() -> [String] {
return dataSource.fetchData()
}
}
Here, RemoteDataSource is speculative generality, added without a clear, immediate requirement.
Solution: Remove or simplify unnecessary abstractions.
class LocalDataSource {
func fetchData() -> [String] {
return ["Local data"]
}
}
class DataManager {
let dataSource = LocalDataSource()
func loadData() -> [String] {
return dataSource.fetchData()
}
}
Simplifying by removing speculative or unnecessary abstractions leads to cleaner, simpler, and more maintainable code. Add complexity only when it’s genuinely needed.
5. Couplers
a) Inappropriate Intimacy
Classes excessively accessing internal details of other classes.
Example:
class Customer {
let name: String
let orders: [Double]
init(name: String, orders: [Double]) {
self.name = name
self.orders = orders
}
}
class ReportGenerator {
func printCustomerReport(customer: Customer) {
let totalAmount = customer.orders.reduce(0, +)
print("Customer \(customer.name) spent a total of \(totalAmount)")
}
}
Here, ReportGenerator accesses data from Customer extensively, indicating the method might belong closer to the customer itself.
Solution: Decouple classes and use clear interfaces.
class Customer {
let name: String
let orders: [Double]
init(name: String, orders: [Double]) {
self.name = name
self.orders = orders
}
func totalOrderAmount() -> Double {
return orders.reduce(0, +)
}
func generateReport() {
print("Customer \(name) spent a total of \(totalOrderAmount())")
}
}
// Usage
let customer = Customer(name: "Alice", orders: [120.0, 35.5, 220.0])
customer.generateReport()
By moving the functionality closer to the relevant data (Customer), the code becomes more cohesive, readable, and maintainable.
b) Message Chains
Excessively long method call chains.
Example:
class Address {
let street: String
init(street: String) {
self.street = street
}
}
class Profile {
let address: Address
init(address: Address) {
self.address = address
}
}
class User {
let profile: Profile
init(profile: Profile) {
self.profile = profile
}
}
// Excessively long message chain
let user = User(profile: Profile(address: Address(street: "123 Main St")))
let street = user.profile.address.street
print("User's street: \(street)")
This chain (user.profile.address.street) is excessively long, decreasing readability.
Solution: Introduce intermediate methods to simplify call chains.
class User {
let profile: Profile
init(profile: Profile) {
self.profile = profile
}
func streetAddress() -> String {
return profile.streetAddress()
}
}
class Profile {
let address: Address
init(address: Address) {
self.address = address
}
func streetAddress() -> String {
return address.street
}
}
class Address {
let street: String
init(street: String) {
self.street = street
}
}
// Simplified usage
let user = User(profile: Profile(address: Address(street: "123 Main St")))
print("User's street: \(user.streetAddress())")
Introducing intermediate methods (streetAddress()) reduces complexity, improves readability, and minimizes potential null-reference errors.
6. Concurrency Smells
a) Temporal Coupling
Methods that must be executed in a specific order.
Example:
class Database {
var isConnected = false
func connect() {
isConnected = true
print("Database connected.")
}
func fetchData() {
guard isConnected else {
print("Error: Database not connected.")
return
}
print("Fetching data...")
}
}
// Usage (Temporal coupling: connect() must be called before fetchData())
let db = Database()
db.connect()
db.fetchData()
Here, fetchData() depends on connect() being called first, creating temporal coupling.
Solution: Refactor methods to make calls independent.
class Database {
private var isConnected = false
private func connectIfNeeded() {
if !isConnected {
isConnected = true
print("Database connected.")
}
}
func fetchData() {
connectIfNeeded()
print("Fetching data...")
}
}
// Usage (No longer order-dependent)
let db = Database()
db.fetchData()
By refactoring the methods to internally manage their dependencies, we eliminate temporal coupling, creating safer and more robust code.
b) Excessive Synchronization
Heavy use of synchronization mechanisms.
Example:
class Counter {
private let lock = NSLock()
private var count = 0
func increment() {
lock.lock()
defer { lock.unlock() }
count += 1
}
func getCount() -> Int {
lock.lock()
defer { lock.unlock() }
return count
}
}
// Usage
let counter = Counter()
counter.increment()
print(counter.getCount())
Here, using NSLock excessively can degrade performance by locking even during simple read operations.
Solution: Employ Swift’s concurrency patterns like actors.
actor Counter {
private var count = 0
func increment() {
count += 1
}
func getCount() -> Int {
return count
}
}
// Usage
let counter = Counter()
Task {
await counter.increment()
let currentCount = await counter.getCount()
print(currentCount)
}
By utilizing granular locking or actor-based concurrency, we significantly improve performance, simplify synchronization logic, and make the code safer and easier to maintain.
7. Readability Issues
a) Poor Naming
Ambiguous or unclear naming of code elements.
Example:
class Data {
var val: Int
init(v: Int) {
val = v
}
func calc() -> Int {
return val * 2
}
}
let obj = Data(v: 10)
print(obj.calc()) // unclear purpose
In this example, the class name (Data), variable (val), and method (calc) are ambiguous, making the code hard to understand.
Solution: Adopt clear, descriptive naming conventions.
class ScoreCalculator {
var baseScore: Int
init(baseScore: Int) {
self.baseScore = baseScore
}
func calculateTotalScore() -> Int {
return baseScore * 2
}
}
let calculator = ScoreCalculator(baseScore: 10)
print(calculator.calculateTotalScore()) // Clearly conveys its purpose
Clear and descriptive naming significantly improves readability, makes intent explicit, and simplifies future maintenance.
b) Magic numbers
Unexplained numeric literals in code.
Example:
func isEligibleForDiscount(orderAmount: Double) -> Bool {
return orderAmount > 100.0 // Magic number: unclear why 100 is used
}
let orderTotal = 120.0
if isEligibleForDiscount(orderAmount: orderTotal) {
print("Discount applied!")
}
Using 100.0 directly in the code is unclear and can confuse readers about its significance.
Solution: Replace with named constants.
func isEligibleForDiscount(orderAmount: Double) -> Bool {
return orderAmount > 100.0 // Magic number: unclear why 100 is used
}
let orderTotal = 120.0
if isEligibleForDiscount(orderAmount: orderTotal) {
print("Discount applied!")
}
Replacing magic numbers with clearly named constants improves readability, maintainability, and makes the code self-explanatory.
c) Deep Nesting
Excessive nesting reducing readability.
Example:
func authenticateUser(username: String?, password: String?) {
if let username = username {
if !username.isEmpty {
if let password = password {
if password.count >= 8 {
print("Authentication successful.")
} else {
print("Password too short.")
}
} else {
print("Password is missing.")
}
} else {
print("Username is empty.")
}
} else {
print("Username is missing.")
}
}
Deep nesting significantly reduces readability and makes it hard to follow logic clearly.
Solution: Refactor using methods, guard clauses, or early returns.
func authenticateUser(username: String?, password: String?) {
guard let username = username, !username.isEmpty else {
print("Username is missing or empty.")
return
}
guard let password = password, password.count >= 8 else {
print("Password is missing or too short.")
return
}
print("Authentication successful.")
}
Refactoring deep nesting into guard clauses or early returns greatly enhances readability, simplifies logic, and clearly states conditions upfront.
8. Exception Handling Smells
a) Swallowed Exceptions
Ignoring errors silently.
Example:
enum FileError: Error {
case notFound
}
func readFile(named fileName: String) throws -> String {
throw FileError.notFound
}
func loadData() {
do {
let data = try readFile(named: "data.txt")
print(data)
} catch {
// Exception is swallowed silently
}
}
loadData()
Here, the exception is caught but ignored, providing no information about the error.
Solution: Explicitly handle errors.
func loadData() {
do {
let data = try readFile(named: "data.txt")
print(data)
} catch {
print("Failed to load data: \(error)")
// Consider logging or handling the error appropriately
}
}
loadData()
Explicitly handling and logging exceptions provides better error visibility, aiding debugging and ensuring issues can be addressed promptly.
b) Broad Exception Catching
Catching overly general exceptions.
Example:
enum NetworkError: Error {
case offline
case timeout
}
func fetchData() throws {
throw NetworkError.timeout
}
do {
try fetchData()
} catch {
print("An error occurred.") // Too general, doesn't specify the actual issue
}
Catching a general exception makes it difficult to diagnose specific problems.
Solution: Catch specific, meaningful exceptions.
do {
try fetchData()
} catch NetworkError.offline {
print("No internet connection. Please check your network.")
} catch NetworkError.timeout {
print("Request timed out. Try again later.")
} catch {
print("An unexpected error occurred: \(error).")
}
Catching specific exceptions clarifies error handling, making debugging easier and providing better user feedback.
c) Excessive Logging
Logging redundant or irrelevant information.
func fetchUserData(userID: Int) {
print("Starting data fetch.")
print("Fetching data for userID: \(userID)")
// Simulated data fetching
print("Data fetch in progress...")
print("Still fetching...")
print("Almost done...")
print("Data fetch completed for userID: \(userID)")
}
fetchUserData(userID: 42)
In this example, multiple redundant logs clutter the output, making it harder to focus on important information.
Solution: Keep logs concise and relevant.
func fetchUserData(userID: Int) {
print("Fetching data for userID: \(userID)...")
// Simulated data fetching
print("Data fetch completed.")
}
fetchUserData(userID: 42)
Keeping logs concise and focused makes debugging easier and improves readability, ensuring only essential information is logged.
9. Architectural Smells
a) Circular Dependencies
Interdependent classes.
Example:
class Engine {
let car: Car
init(car: Car) {
self.car = car
}
}
class Car {
var engine: Engine?
init() {
self.engine = Engine(car: self)
}
}
In this example, Car depends on Engine, and Engine also depends on Car, creating a circular dependency.
Solution: Refactor to Remove Circular Dependency using Protocols.
protocol CarProtocol {
func start()
}
class Engine {
weak var car: CarProtocol?
init(car: CarProtocol?) {
self.car = car
}
func ignite() {
print("Engine started.")
car?.start()
}
}
class Car: CarProtocol {
var engine: Engine?
init() {
self.engine = Engine(car: self)
}
func start() {
print("Car started.")
}
}
// Usage
let car = Car()
car.engine?.ignite()
By using a protocol (CarProtocol) and a weak reference, the circular dependency is effectively removed, leading to more modular, testable, and maintainable code.
b) Layer Violation
Bypassing established layers in application architecture.
Example:
// Presentation Layer directly accessing Data Layer
class ViewController {
func loadData() {
let database = Database()
let data = database.fetchData() // Direct database access (layer violation)
print(data)
}
}
class Database {
func fetchData() -> [String] {
return ["Data item 1", "Data item 2"]
}
}
Solution: Enforce layer boundaries strictly.
// Data Layer
class Database {
func fetchData() -> [String] {
return ["Data item 1", "Data item 2"]
}
}
// Business Logic Layer
class ViewModel {
private let database = Database()
func getDataForView() -> [String] {
return database.fetchData()
}
}
// Presentation Layer
class ViewController {
private let viewModel = ViewModel()
func loadData() {
let data = viewModel.getDataForView() // Access via intermediate layer
print(data)
}
}
Strictly enforcing layer boundaries through proper abstraction (like ViewModel) improves maintainability, reduces tight coupling, and preserves the intended architecture.
c) God Object
Single classes dominating multiple responsibilities.
Example:
class UserManager {
func authenticateUser(username: String, password: String) {
// authentication logic
}
func saveUserProfile(userData: [String: Any]) {
// save profile logic
}
func sendNotification(message: String) {
// notification logic
}
func logActivity(activity: String) {
// activity logging logic
}
}
In this example, UserManager handles authentication, data persistence, notifications, and logging—making it a God Object.
Solution: Refactor into smaller, focused classes.
class AuthenticationService {
func authenticate(username: String, password: String) {
// authentication logic
}
}
class UserProfileService {
func saveProfile(userData: [String: Any]) {
// save profile logic
}
}
class NotificationService {
func sendNotification(message: String) {
// notification logic
}
}
class ActivityLogger {
func log(activity: String) {
// logging logic
}
}
// Usage
let authService = AuthenticationService()
authService.authenticate(username: "user", password: "1234")
let profileService = UserProfileService()
profileService.saveProfile(userData: ["name": "John"])
let notificationService = NotificationService()
notificationService.sendNotification(message: "Profile updated")
let logger = ActivityLogger()
logger.log(activity: "User authenticated")
Refactoring a God Object into smaller, dedicated classes enhances readability, simplifies code maintenance, and adheres to the Single Responsibility Principle.
10. Testing Smells
a) Fragile Tests
Tests breaking easily with minor changes.
Example:
// Implementation
class Calculator {
private(set) var total = 0
func add(_ value: Int) {
total += value
}
}
// Fragile Test (depends on internal implementation detail `total`)
func testCalculatorAdd() {
let calculator = Calculator()
calculator.add(5)
assert(calculator.total == 5) // fragile, tied to internal state
}
This test directly checks internal state, making it fragile if the implementation detail (total) changes.
Solution: Test Behaviors and Interfaces, not Implementation Details.
// Improved Implementation
class Calculator {
private var total = 0
func add(_ value: Int) {
total += value
}
func currentTotal() -> Int {
return total
}
}
// Robust Test (testing behavior through public interfaces)
func testCalculatorAdd() {
let calculator = Calculator()
calculator.add(5)
assert(calculator.currentTotal() == 5) // test interface, not internal details
}
Testing against clearly defined behaviors and public interfaces makes tests robust, less sensitive to minor changes, and easier to maintain.
b) Slow Tests
Long-running tests reducing efficiency.
Example:
func testUserServiceFetchesUser() {
let userService = UserService()
let expectation = XCTestExpectation(description: "Fetch user")
userService.fetchUser(userID: 42) { user in
XCTAssertEqual(user.name, "John Doe")
expectation.fulfill()
}
wait(for: [expectation], timeout: 5) // Slow test due to actual network request
}
This test is slow and unreliable because it depends on a live network request.
Solution: Optimize or mock external dependencies.
// Mock UserService to avoid real network calls
class MockUserService: UserServiceProtocol {
func fetchUser(userID: Int, completion: (User) -> Void) {
let mockUser = User(id: userID, name: "John Doe")
completion(mockUser)
}
}
func testUserServiceFetchesUserQuickly() {
let mockService = MockUserService()
let expectation = XCTestExpectation(description: "Fetch user quickly")
mockService.fetchUser(userID: 42) { user in
XCTAssertEqual(user.name, "John Doe")
expectation.fulfill()
}
wait(for: [expectation], timeout: 1) // Fast and reliable
}
By using mock services or stubs, we eliminate external dependencies, significantly speeding up tests and improving reliability and efficiency.
c) Test Duplication
Repetitive test code.
Example:
func testUserInitialization() {
let user = User(name: "Alice", age: 30)
XCTAssertEqual(user.name, "Alice")
XCTAssertEqual(user.age, 30)
}
func testUserAgeUpdate() {
let user = User(name: "Alice", age: 30)
user.updateAge(to: 31)
XCTAssertEqual(user.age, 31)
}
Here, the same initialization (User(name: "Alice", age: 30)) is repeated.
Solution: Extract common test logic into reusable methods or fixtures.
var testUser: User!
override func setUp() {
super.setUp()
testUser = User(name: "Alice", age: 30)
}
func testUserInitialization() {
XCTAssertEqual(testUser.name, "Alice")
XCTAssertEqual(testUser.age, 30)
}
func testUserAgeUpdate() {
testUser.updateAge(to: 31)
XCTAssertEqual(testUser.age, 31)
}
By extracting repeated test setups into reusable fixtures or methods (setUp()), we reduce redundancy, simplify tests, and make them easier to maintain.
d) Missing Test Coverage
Lack of tests for critical code paths.
Example:
class PaymentProcessor {
func process(amount: Double) throws {
guard amount > 0 else {
throw PaymentError.invalidAmount
}
// Processing logic...
}
}
enum PaymentError: Error {
case invalidAmount
}
// Existing test (covers only the positive case)
func testProcessPaymentValidAmount() throws {
let processor = PaymentProcessor()
XCTAssertNoThrow(try processor.process(amount: 50))
}
Here, there’s missing test coverage for the negative scenario (invalid payment amounts), leaving critical logic untested.
Solution: Regularly review and expand test coverage.
func testProcessPaymentInvalidAmount() throws {
let processor = PaymentProcessor()
XCTAssertThrowsError(try processor.process(amount: -10)) { error in
XCTAssertEqual(error as? PaymentError, PaymentError.invalidAmount)
}
}
Expanding test coverage ensures all critical paths are validated, improving reliability and reducing potential errors.
Conclusion
Understanding and addressing code smells proactively helps maintain clean, efficient, and manageable Swift codebases. By recognizing common issues like the above ones, we can implement targeted refactoring strategies to enhance code quality.
Sharing is caring!