Nawet jeśli nie ma żadnych testów, to i tak należy stosować SRP.
SRP jest OK, ale często ciężko sensownie określić co jest jedną odpowiedzialnością. SRP traktowałbym raczej jako pomocną zasadę przy refaktoryzacji, podziale rozrastającej się klasy na mniejsze części, niż jako żelazną regułę stosowaną nawet jak klasy są mikroskopijne i dzielenie ich dalej jest nieopłacalne. Nadgorliwość gorsza od faszyzmu.
Podam przykład by lepiej zobrazować co miałem na myśli pisząc o nadpisywaniu metod i unikaniu dodatkowych hierarchii klas (obojętnie czy hierarchia kompozycji czy dziedziczenia).
Klasa produkcyjna (są 3 typy mejli, podobnie jak np w pewnym "dużym mikroserwisie" u nas w pracy i tego się trzymajmy):
import org.apache.commons.mail.SimpleEmail
object SecretIds {
def hash(data: String, secret: String): Int =
(secret + data).hashCode // TODO replace with sth stable
}
class AppMailer(smtpHostName: String,
smtpPort: Int,
smtpUsername: String,
smtpPassword: String,
appBaseUrl: String,
secret: String,
feedbackEmail: String) {
def sendPasswordValidationEmail(userId: Long, userEmail: String): Unit = {
val checksum = SecretIds.hash(userEmail, secret)
sendEmail("my-app@no-reply",
userEmail,
"Password validation",
s"To validate password go to: " +
s"$appBaseUrl/password/validate/$userId/$checksum")
}
def sendPasswordRecoveryEmail(userId: Long, userEmail: String): Unit = {
val checksum = SecretIds.hash(userEmail, secret)
sendEmail("my-app@no-reply",
userEmail,
"Password recovery",
s"To recover password go to: " +
s"$appBaseUrl/password/validate/$userId/$checksum")
}
def sendFeedbackEmail(userEmail: String, feedback: String): Unit = {
sendEmail("my-app@no-reply",
feedbackEmail,
s"Feedback from user $userEmail",
s"Feedback: \n$feedback")
}
protected def sendEmail(fromAddress: String,
toAddress: String,
subject: String,
content: String): Unit = {
val email = new SimpleEmail()
email.setHostName(smtpHostName)
email.setSmtpPort(smtpPort)
email.setAuthentication(smtpUsername, smtpPassword)
email.setFrom(fromAddress)
email.addTo(toAddress)
email.setSubject(subject)
email.setMsg(content)
email.send()
}
}
Klasa testowa:
import org.scalatest.{FlatSpec, MustMatchers}
class AppMailerSpec extends FlatSpec with MustMatchers {
behavior of "AppMailer"
it must "send password validation email" in {
val mailer = new TestMailer
val eAddress = "user1@email.com"
mailer.sendPasswordValidationEmail(34, eAddress)
val secretId = SecretIds.hash(eAddress, "secret")
mailer.lastEmails mustBe Seq(
Email("my-app@no-reply",
eAddress,
"Password validation",
"To validate password go to: " +
s"base URL/password/validate/34/$secretId"))
}
it must "send password recovery email" in {
val mailer = new TestMailer
val eAddress = "bob@smith.org"
mailer.sendPasswordRecoveryEmail(78, eAddress)
val secretId = SecretIds.hash(eAddress, "secret")
mailer.lastEmails mustBe Seq(
Email("my-app@no-reply",
eAddress,
"Password recovery",
"To recover password go to: " +
s"base URL/password/validate/78/$secretId"))
}
it must "send feedback" in {
val mailer = new TestMailer
val eAddress = "john@smith.com"
mailer.sendFeedbackEmail(eAddress, "I am happy")
mailer.lastEmails mustBe Seq(
Email("my-app@no-reply",
"feedback@email.com",
s"Feedback from user $eAddress",
"Feedback: \nI am happy"))
}
case class Email(fromAddress: String,
toAddress: String,
subject: String,
content: String)
class TestMailer
extends AppMailer(null,
0,
null,
null,
"base URL",
"secret",
"feedback@email.com") {
private[AppMailerSpec] var lastEmails = Vector.empty[Email]
override protected def sendEmail(fromAddress: String,
toAddress: String,
subject: String,
content: String): Unit = {
lastEmails :+= Email(fromAddress, toAddress, subject, content)
}
}
}
Jak widać z powyższego kodu:
- w kodzie produkcyjnym mamy jedną klasę - AppMailer, więc nie ma mowy o żadnej hierarchii
- podklasa jest w testach, ale jest to podklasa lokalna dla speca
- w tej testowej podklasie nadpisuję nieabstrakcyjną metodę z efektami ubocznymi - podmieniam jedne efekty uboczne (ciężko testowalne) na inne (łatwo testowalne)
- logika produkcyjna jest trywialna, a logika testowa nie jest wcale kiepska
Można cudować i np dzielić klasę produkcyjną na mniejsze klasy, ale nie widzę w tym przypadku żadnego zysku z tego.