Immer wieder steht man vor der Herausforderung, zu einem bestehenden Legacy-Code neue Funktionalitäten hinzu zufügen oder vorhandene Bugs beheben zu müssen.
Ich möchte hier eine Möglichkeit dazu beschreiben, einen Weg den ich u.a. von Michael Feathers gelernt habe.
Nehmen wir an, der Code schaut so aus:
using System.Net.Mail;
namespace LegacyCode
{
public class KompletterWorkflow
{
public void SpeichereDatenUndVersendeMail( int id, string emailaddress, string text, string cc )
{
if( !string.IsNullOrEmpty( text ) )
{
Daten user;
DatenRepository repository = new DatenRepository();
user = new Daten { Id = id, Email = emailaddress };
repository.Save( user );
if( !SmtpRepository.IsValidEmailAddress( emailaddress ) ) return;
IEmailRepository emailrep = new SmtpRepository();
MailMessage message = new MailMessage( "myAddress@xy.com", emailaddress, "Testmail", text );
if( !string.IsNullOrEmpty( cc ) ) message.CC.Add( cc );
emailrep.SendMail( message );
}
}
}
}
Die referenzierten Klassen wären dann sowas wie:
public class DatenRepository
{
public void Save( Daten daten )
{
//Speichert die Daten ab
}
}
public interface IEmailRepository
{
void SendMail( MailMessage message );
}
public class SmtpRepository : IEmailRepository
{
public static bool IsValidEmailAddress( string address )
{
try
{
MailAddress mailAddress = new MailAddress( address );
return true;
}
catch
{
return false;
}
}
public void SendMail( MailMessage message )
{
//Versende die Email
}
}
public class Daten
{
public int Id { get; set; }
public string Email { get; set; }
}
Diese Klasse, so wie sie ist, verletzt jede Menge Prinzipien.
Die wichtigsten sind: The Single Responsibility Principle (SRP) und The Open-Closed Principle (OCP). Außerdem ist die Klasse in diesem Zustand nicht testbar, da man mittendrin Objekte erstellt, deren Verhalten auch mitgetestet werden müssten.
Wenn man das weißt, dann ist die Aufgabe einfach: die Implementierungen DatenRepository und SmtpRepository dürfen nicht in der Methode instanziert werden, sondern sie müssen nach der Regeln der Inversion of Control (IoC) der Klasse übergeben werden.
Zusätzlich werden auch die zusammenhängenden Parameter der Methode SpeichereDatenUndVersendeMail zum userDaten zusammengefasst.
public class KompletterWorkflow
{
private DatenRepository m_repository;
private IEmailRepository m_emailrep;
public KompletterWorkflow(DatenRepository repository, IEmailRepository emailrep)
{
m_repository = repository;
m_emailrep = emailrep;
}
public void SpeichereDatenUndVersendeMail( Daten userDaten, string text, string cc )
{
if( !string.IsNullOrEmpty( text ) )
{
m_repository.Save( userDaten);
if( !SmtpRepository.IsValidEmailAddress( userDaten.Email ) ) return;
MailMessage message = new MailMessage( "myAddress@xy.com", userDaten.Email, "Testmail", text );
if( SmtpRepository.IsValidEmailAddress( cc ) ) message.CC.Add( cc );
m_emailrep.SendMail( message );
}
}
}
Um SRP gerecht zu werden, müsste man auch die Methode in 2 teilen: DatenSpeichern und VersendeMail. Das ist allerdings aus der Sicht der Testbarkeit unwichtig und ich will den Artikel nicht unnötig in die Länge ziehen 😉
Jetzt müssen wir nur noch die Tests schreiben.
Da wir Unittests schreiben, also nur das Verhalten dieser eine Methode testen, müssen wir die fremden Objekte mocken: Das hbedeutet, wir werden ihr Verhalten nachspielen, so tun als ob.
Wir müssen 3 verschiedene Fälle behandeln: ein Interface IEmailRepository, eine konkrete Implementierung DatenRepository und eine statische Methode SmtpRepository.IsValidEmailAddress(string). Bei den ersten zwei empfehlt es sich ein Mocking-Framework wie z.B. RhinoMock zu nutzen.
using NUnit.Framework;
using Rhino.Mocks;
using System.Net.Mail;
namespace LegacyCode.Tests
{
[TestFixture]
public class DatenSpeichernUndVersendeMailTests
{
private IEmailRepository m_emailrep;
private DatenRepository m_repository;
[SetUp]
public void Init()
{
m_emailrep = MockRepository.GenerateStub<IEmailRepository>();
m_repository = MockRepository.GenerateStub<DatenRepository>();
}
[Test]
public void Daten_werden_gespeichert_wenn_Text_nicht_leer()
{
//Arrange
KompletterWorkflow workflow = new KompletterWorkflow( m_repository, m_emailrep );
Daten userDaten = new Daten{Id=1,Email="test@test.de"};
//Act
workflow.SpeichereDatenUndVersendeMail( userDaten , "Emailtext", "cc" );
//Assert
m_repository.AssertWasCalled( a => a.Save( userDaten ) );
m_emailrep.AssertWasCalled( a => a.SendMail( Arg<MailMessage>.Matches( b => b.To[0].Address == userDaten.Email && b.CC.Count == 0 ) ) );
}
}
}
Wenn wir jetzt die Tests mit NUnit ausführen, bekommen wir folgende Fehlermeldung:
LegacyCode.Tests.DatenSpeichernUndVersendeMailTests.Daten_werden_gespeichert_wenn_Text_nicht_leer:
System.InvalidOperationException : No expectations were setup to be verified, ensure that the method call in the action is a virtual (C#) / overridable (VB.Net) method call
Das heißt, die Methoden, worüber wir Annahmen treffen, müssen entweder in einem Interface veröffentlicht worden oder überschreibbar also virtual sein.
public virtual void Save( Daten daten )
{
//Speichert die Daten ab
}
Wenn wir jetzt die Tests durchführen, dann passt alles.
Für die statische Methode erstellt man am besten eine separate Testklasse. Da hier keine andere Komponenten wie Datenbank oder Filesystem angesprochen werden, wird die Methode ganz einfach zu testen sein:
[TestFixture]
public class IsValidEmailTests
{
[Test]
public void Leere_Adresse_ist_nicht_valide()
{
Assert.That( SmtpRepository.IsValidEmailAddress( string.Empty ), Is.False );
}
}
Das war’s !
Mit weiteren Tests kann das Verhalten unserer Klasse komplett abgedeckt und danach mit TDD die Klasse erweitert oder verändert werden.
Und das alles ohne die Befürchtung, dass diese Änderungen zu unvorhersehbaren Ergebnissen führen.
