(Note au utilisateur de GoogleReader : Pour une raison inconnue, les couleurs ne sortent pas … lisez plutôt ici)
Je souhaite vous montrer dans ce billet un exemple d’ajout de test unitaire dans du legacy code. Du legacy code, c’est du code non testé automatiquement. Je travaille tous les jours sur du code non couvert par des tests automatiques alors capitaliser en ajoutant petit à petit des tests est un réel enjeu pour éviter les régressions.
Le Problème
Voici une classe développée il y a presque 10 ans pour dessiner des annotations sur un graphique. Il se trouve que dans certaines rares conditions, l’annotation sort du graphique et ceci ne fait pas propre lors de son impression. Nous avons donc souhaité corriger ce défaut.
__________________________________________________________________
TSKRAnnotDrawer = class
private
FAnnotList: TList;
FSquaring: TSquaringType;
FHorizontalMaille: Real;
FVerticalMaille: Real;
procedure Sort;
function GetAnnot(Index: Integer): PAnnotRec;
procedure FindCorrespondingSquaringRectangle(aRect: TRect;
X, Y: Integer;
var SquaringRow, SquaringCol: Integer);
procedure CalculateSquaringRectanglesNumber(aRect: TRect;
W, H: Integer; var SquaringWidth, SquaringHeight: Integer);
function FindFreeSpace(SquaringRow, SquaringCol,
SquaringWidth, SquaringHeight: Integer;
var SquaringFreeRow, SquaringFreeCol: Integer;
var UpOrDown: Boolean): Boolean;
function TestIfItIsInTheGraphic(row, col,
SquaringWidth, SquaringHeight: Integer): Boolean;
function TestIfTheSpaceIsFree(row, col,
SquaringWidth, SquaringHeight: Integer): Boolean;
procedure UpdateFSquaring(row, col, SquaringWidth, SquaringHeight: Integer);
public
PeakAnnotation: TSKRPeakAnnotationPrefs;
EventAnnotation: TSKREventAnnotationPrefs;
procedure Create;
procedure Destroy; override;
procedure ClearList;
procedure AddToList(aAnnot: TAnnotRec);
procedure Draw(Canvas: TCanvas3D; arect: TRect);
end;
__________________________________________________________________
Nous allons nous intéresser à la méthode “Draw”. Après investigation, l’erreur provient du calcul de la hauteur de la première annotation. En effet, la ‘font size’ est assignée après le calcul de la hauteur du texte : voir ci-dessous le texte en rouge.
__________________________________________________________________
procedure TSKRAnnotDrawer.Draw(Canvas: TCanvas3D; aRect: TRect);
var
i, Width, Height: Integer;
UpOrDown: Boolean;
w, h: Integer;
AnnotationPrefs: TSKRAnnotation;
row, col: Integer;
SquaringRow, SquaringCol: Integer;
SquaringWidth, SquaringHeight: Integer;
SquaringFreeRow, SquaringFreeCol, XFree, YFree, YFreeTemp: Integer;
begin
assert(EventAnnotation <> nil);
assert(PeakAnnotation <> nil);
// sort annotation according time
Sort;
Width := 0;
Height := -1;
//initializing the squaring array.
for row := 0 to Pred(SquaringSize) do // Iterate
begin
for col := 0 to Pred(SquaringSize) do // Iterate
begin
FSquaring[row, col] := True;
end; // for
end; // for
for i := 0 to Pred(FAnnotList.Count) do
with GetAnnot(i)^ do
begin
if Typ = 0 then
AnnotationPrefs := EventAnnotation
else
AnnotationPrefs := PeakAnnotation;
UpOrDown := AnnotationPrefs.AutoUpDown = apUp;
if AnnotationPrefs.Orientation = aoOblique then
begin
//We take the size of the rectangle projection on a vertical axis
H := round(Sin(Pi / 4) * (Canvas.TextWidth(Text) + Canvas.TextHeight(Text)));
//and for W the size of the rectangle projection on the horizontal axis
W := round(Cos(Pi / 4) * (Canvas.TextWidth(Text) + Canvas.TextHeight(Text)));
end
else if AnnotationPrefs.Orientation = aoVertical then
begin
H := Canvas.TextWidth(Text);
W := Canvas.TextHeight(Text);
end
else //Horizontal
begin
H := Canvas.TextHeight(Text);
W := Canvas.TextWidth(Text);
end;
H := H + 4;
if AnnotationPrefs.AutoUpDown = apAuto then
begin
Canvas.Font.Name := AnnotationPrefs.Font.name;
Canvas.Font.Size := AnnotationPrefs.Font.Size; //INTERVIENT TROP TARD
end;
if AnnotationPrefs.LinkType = ltNone then
begin
... 6 lines
end
else
begin
... 30 lines
end;
DrawAnnotation(Canvas, Text, Point(XFree, YFree),
XFree, AnnotationPrefs, UpOrDown, Width, Height, X, Y, aRect);
end;
end;
__________________________________________________________________
La correction est triviale, il suffit de remonter l’affectation de la font size avant la première utilisation du Canvas.
Mais comment tester ce que l’on souhaite modifier?
Le cheminement vers le test unitaire
Ma première idée fut d’instancier la classe et appeler la méthode Draw dans un test unitaire. En effet, le constructeur ne prend pas de paramètre alors ça paraissait facile. Ensuite, il a fallu ajouter des assertions à mon test et là les ennuis commençaient. En effet, je n’avais aucune envie de vérifier le rendu sur le graphique (via le Canvas) car je ne savais pas comment m’y prendre.
Donc je suis reparti de zéro afin d’isoler mon problème. J’ai suivi les étapes suivantes :
1- Extraction de la partie de code problèmatique
Je ne prend pas de risque à ce niveau là et je fais confiance au compilateur pour m’indiquer les éventuels problèmes.
__________________________________________________________________
procedure TSKRAnnotDrawer.getTextHeightAndWidth(
const aCanvas: TCanvas3D;
const aAnnotationPrefs: TSKRAnnotation;
const aText: string;
var oTextHeight, oTextWidth: integer);
begin
if AnnotationPrefs.Orientation = aoOblique then
begin
//We take the size of the rectangle projection on a vertical axis
H := round(Sin(Pi / 4) * (Canvas.TextWidth(Text) + Canvas.TextHeight(Text)));
//and for W the size of the rectangle projection on the horizontal axis
W := round(Cos(Pi / 4) * (Canvas.TextWidth(Text) + Canvas.TextHeight(Text)));
end
else if AnnotationPrefs.Orientation = aoVertical then
begin
H := Canvas.TextWidth(Text);
W := Canvas.TextHeight(Text);
end
else //Horizontal
begin
H := Canvas.TextHeight(Text);
W := Canvas.TextWidth(Text);
end;
H := H + 4;
if AnnotationPrefs.AutoUpDown = apAuto then
begin
Canvas.Font.Name := AnnotationPrefs.Font.name;
Canvas.Font.Size := AnnotationPrefs.Font.Size; //INTERVIENT TROP TARD
end;
end;
__________________________________________________________________
Là, ça devient plus simple de tester cette fonction mais celle-ci n’est pas publique et changer l’API, c’est à dire le contrat de la classe, à des fins de tests n’est pas élegant. En effet, cette méthode n’a pas d’intéret pour l’utilisateur de la classe.
2- Extraction de la méthode vers une classe spécifique
En ayant en tête les principes SOLID et plus particulièrement celui qui dit qu’une classe ne doit avoir qu’une responsabilité, je n’ai pas d’état d’âme à créer une classe pour isoler ma méthode:
__________________________________________________________________
TTextPixelSizeHelper = class
public
class procedure GetTextHeightAndWidth(
const aCanvas: TCanvas3D;
const aAnnotationPrefs: TSKRAnnotation;
const aText: string;
var oTextHeight, oTextWidth: integer);
end;
class procedure TTextPixelSizeHelper .getTextHeightAndWidth(
const aCanvas: TCanvas3D;
const aAnnotationPrefs: TSKRAnnotation;
const aText: string;
var oTextHeight, oTextWidth: integer);
begin
... idem ...
end;
__________________________________________________________________
Je ne prend toujours pas de risque et je fais confiance au compilateur pour les éventuelles erreurs de refactoring. Je n’ai changé aucun comportement.
La méthode Draw peu maintenant utiliser cette nouvelle classe :
__________________________________________________________________
procedure TSKRAnnotDrawer.Draw(Canvas: TCanvas3D; aRect: TRect);
var
...
begin
...
for i := 0 to Pred(FAnnotList.Count) do
with GetAnnot(i)^ do
begin
if Typ = 0 then
AnnotationPrefs := EventAnnotation
else
AnnotationPrefs := PeakAnnotation;
UpOrDown := AnnotationPrefs.AutoUpDown = apUp;
TTextPixelSizeHelper.GetTextHeightAndWidth(Canvas, AnnotationPrefs, Text, H, W);
...
DrawAnnotation(Canvas, Text, Point(XFree, YFree),
XFree, AnnotationPrefs, UpOrDown, Width, Height, X, Y, aRect);
end;
end;
__________________________________________________________________
Au passage, nous avons diminué la complexité cyclomatique de la fonction et facilité sa compréhension.
3- Ecriture d’un test qui échoue
Le principe du test est d’appeler deux fois de suite la fonction ‘GetTextHeightAndWidth’ avec le même texte et de vérifier qu’elle donne deux fois le même résultat.
__________________________________________________________________
procedure TSKRAnnotDrawerTest.testPeakNamesMightBePrintedBeOutisdeOfTheChartArea;
const
TEXT1_TO_DRAW = 'TestPeakAnnot';
TEXT2_TO_DRAW = TEXT1_TO_DRAW;
PEAK_ANNOTATION_TYPE = 1;
var
chart: TOrlandoChart;
form: TForm;
canvas: TCanvas3D;
peak_annotation_prefs: TSKRPeakAnnotationPrefs;
text1_height_result, text2_height_result: integer;
text1_width_result, text2_width_result: integer;
begin
peak_annotation_prefs := TSKRPeakAnnotationPrefs.Create;
form := TForm.Create(nil);
chart := TOrlandoChart.Create(form);
chart.ParentWindow := form.Handle;
canvas := chart.Canvas;
try
canvas.Font.Size := 4;
assert(canvas.Font.Size <> peak_annotation_prefs.Font.size, 'Font size must be different to show the defect problem');
TTextPixelSizeHelper.GetTextHeightAndWidth(
canvas, peak_annotation_prefs, TEXT1_TO_DRAW,
text1_height_result, text1_width_result);
TTextPixelSizeHelper.GetTextHeightAndWidth(
canvas, peak_annotation_prefs, TEXT2_TO_DRAW,
text2_height_result, text2_width_result);
CheckEquals(text1_height_result, text2_height_result,
'calling twice the function with same text should provide the same Height result!');
CheckEquals(text1_width_result, text2_width_result,
'calling twice the function with same text should provide the same Width result!');
finally
peak_annotation_prefs.Free;
chart.Free;
form.free;
end;
end;
_________________________________________________________________
4- Correction du bug
La correction conciste à déplacer quelques lignes de code afin d’assigner la “font size” du canvas avant de s’en servir:
_________________________________________________________________
procedure TTextPixelSizeHelper.getTextHeightAndWidth(
const aCanvas: TCanvas3D;
const aAnnotationPrefs: TSKRAnnotation;
const aText: string;
var oTextHeight, oTextWidth: integer);
begin
if AnnotationPrefs.AutoUpDown = apAuto then
begin
Canvas.Font.Name := AnnotationPrefs.Font.name;
Canvas.Font.Size := AnnotationPrefs.Font.Size;
end;
if AnnotationPrefs.Orientation = aoOblique then
begin
//We take the size of the rectangle projection on a vertical axis
H := round(Sin(Pi / 4) * (Canvas.TextWidth(Text) + Canvas.TextHeight(Text)));
//and for W the size of the rectangle projection on the horizontal axis
W := round(Cos(Pi / 4) * (Canvas.TextWidth(Text) + Canvas.TextHeight(Text)));
end
else if AnnotationPrefs.Orientation = aoVertical then
begin
H := Canvas.TextWidth(Text);
W := Canvas.TextHeight(Text);
end
else //Horizontal
begin
H := Canvas.TextHeight(Text);
W := Canvas.TextWidth(Text);
end;
H := H + 4;
end;
_________________________________________________________________
La barre est verte! Donc place au refactoring si nécéssaire.
Conclusion
Tester du legacy code peut décourager bon nombre de développeurs. Ce n’ai pas très compliqué mais ça demande une petite gymnastique intellectuelle. Tout m’a semblé plus facile après quelques essais et la lecture du bien connu livre de Michael C. Feathers “Working Effectively with Legacy Code” . Ce n’ai pas toujours possible à un coût raisonnable alors dans notre équipe, nous ne nous demandons pas une obligation de résultat.
Avec quelques amis développeurs Grenoblois, nous allons présenter une session dédiée à ce sujet sur du vrai code lors de la conférence Agile Grenoble 2010; venez nous voir ;o) !
Aucun commentaire:
Enregistrer un commentaire