Improving code quality - Session 33: Chef's whimsical decoration

The original article was published on July 11, 2024.

Hello, I'm Munetoshi Ishikawa, a mobile client developer for the LINE messaging app.

This article is the latest installment of our weekly series "Improving code quality". For more information about the Weekly Report, please see the first article.

Chef's whimsical decoration

Let's say you are creating an application for sending and receiving messages. The following MessageModel is the data model for the messages to be sent and received, and MessageRemoteClient is the class for sending and receiving with the server.

class MessageModel(
    val messageText: String,
    val timestampMillis: Long,
    ...
)

class MessageRemoteClient(...) {
    fun sendMessage(messageModel: MessageModel): SendResult { ... }

    ...
}

Suppose the specification has changed to encrypt MessageModel.messageText (such as end-to-end encryption). One option is to implement this encryption within MessageRemoteClient. However, if there are other classes using MessageModel in the same layer, there is a possibility of forgetting to implement encryption. For example, the following MessageRemoteClient performs message encryption, but MessageBackupClient forgets to apply encryption.

class MessageRemoteClient(...) {
    fun sendMessage(messageModel: MessageModel): SendResult {
        ...
        val encryptedMessageText = encrypt(messageModel.messageText, ...)
        val encryptedMessageModel = MessageModel(encryptedMessageText, ...)

        return querySendMessage(encryptedMessageModel, ...)
    }
}

class MessageBackupClient(...) {
    fun backupAllMessages(messageModels: Collection<MessageModel>): BackupResult {
        ...
        
        val responses = messageModels.asSequence()
            .chunked(...)
            .map { queryMessageBackup(it) }

        return ...
    }
}

To avoid such a situation, a developer applied the Decorator pattern to MessageModel. The "encrypted" message is represented as EncryptedMessageModel.

open class MessageModel(
    open val messageText: String,
    val timestampMillis: Long,
    ...
)

class EncryptedMessageModel(
    original: MessageModel,
    encryptionData: EncryptionData
) : MessageModel(
    encrypt(original.messageText, encryptionData),
    original.timestampMillis,
    ...
) {
    companion object {
        private fun encrypt(
            text: String,
            encryptionData: EncryptionData
        ) : String { ... }
    }
}

class MessageRemoteClient(...) {
    fun sendMessage(messageModel: MessageModel): SendResult { ... }
}

With this code, even if you implement multiple ...Client, the encryption logic is centralized in EncryptedMessageModel. However, this change has caused another problem. What is it?

Unordered decoration

When using the Decorator pattern, make sure the following two points are met:

1. The user of the object does not need to worry about whether the Decorator is present or not.
2. The Decorator can be applied any number of times, in any order, and in any combination.

EncryptedMessageModel does not meet these conditions. On the ...Client side, whether it is encrypted or not is important information and needs to be distinguished. Of course, if you change the parameter type of sendMessage to EncryptedMessageModel, you can prevent receiving plain MessageModel, but then there is no need to use the Decorator pattern. Also, if the layer where encryption is applied becomes ambiguous, it can cause bugs where encryption is applied multiple times.

In this case, there is no need to use the Decorator pattern. It is sufficient to define separate models for plaintext and ciphertext and provide utility functions for encryption/decryption separately. In the following implementation, by making the constructor private, you cannot freely create an instance of the encrypted model, and instead, you can obtain it using fromPlainTextMessage. This way, you can avoid bugs that unintentionally encrypt multiple times.

class PlainTextMessageModel(
    val messageText: String,
    val timestampMillis: Long,
    ...
)

class EncryptedMessageModel private constructor(
    val encryptedMessageText: String,
    val timestampMillis: Long,
    ...
) {
    companion object {
        private fun fromPlainTextMessage(
            plainTextMessageModel: PlainTextMessageModel
        ): EncryptedMessageModel { ... }
    }
}

class MessageRemoteClient(...) {
    fun sendMessage(messageModel: EncryptedMessageModel): SendResult { ... }
}

When separating types like this, if there are many common properties, consider extracting them as another data model. In the following implementation, common properties are extracted as MessageMetadata, and PlainTextMessageModel and EncryptedMessageModel have them. (Of course, assuming that properties other than messageText are not encrypted.)

class MessageMetadata(
    val timestampMillis: Long,
    ...
)

class PlainTextMessageModel(
    val messageText: String,
    val messageMetadata: MessageMetadata
)

class EncryptedMessageModel private constructor(
    val encryptedMessageText: String,
    val messageMetadata: MessageMetadata
) { ... }

In a nutshell

When using the Decorator pattern, check its conditions.

Keywords: decorator pattern, type checking, data model

Read other articles on techniques for improving code quality

List of articles on techniques for improving code quality