CodeSOD: Good Etiquette

"Here, you're a programmer, take this over. It's business critical."

That's what Felicity's boss told her when he pointed her to a network drive containing an Excel spreadsheet. The Excel spreadsheet contained a pile of macros. The person who wrote it had left, and nobody knew how to make it work, but the macros in question were absolutely business vital.

Also, it's in French.

We'll take this one in chunks. The indentation is as in the original.

Public Sub ExporToutVersBaseDonnees(ClasseurEnCours As Workbook)Call AffectionVariables(ToutesLesCellulesNommees)Call AffectationBaseDonnees(BaseDonnees)BaseDonnees.Activate

The procedures AffectionVariables and AffectationBaseDonnees populate a pile of global variables. "base de donnees" is French for database, but don't let the name fool you- anything referencing "base de donnees" is referencing another Excel file located on a shared server. There are, in total, four Excel files that must live on a shared server, and two more which must be in a hard-coded path on the user's computer.

Oh, and the shared server is referenced not by a hostname, but by IP address- which is why the macros were breaking on everyone's computer; the IP address changed.

Let's continue.

'Verifier si la ligne existe deja. If ClasseurEnCours.Sheets("DATA").Range("Num_Fichier") = 0 Then Num_Fichier = BaseDonnees.Sheets(1).Range("Dernier_Fichier").Value + 1Inserer_Ligne: '(etiquette Goto) inserer une ligne Application.GoTo Reference:="Derniere_Ligne" Selection.EntireRow.Insert'Copie les cellules (colonne A a colonne FI) de la ligne au-dessus de la ligne inseree. With ActiveCell .Offset(-1, 0).Range("A1:FM1").Copy'Colle le format de la cellule precedemment copiee a la cellule active puis libere les donnees du presse papier .PasteSpecial .Range("A1:FM1").Value = ""'Se repositionne au debut de la ligne inseree. .Range("A1").Select End With Application.CutCopyMode = False

Uh oh, Inserer_Ligne is a label for a Goto target. Not to be confused by the Application.GoTo call on the next line- that just selects a range in the spreadsheet.

After that little landmine, we copy/paste some data around in the sheet.

That's the If side of the conditional, let's look at the else clause:

 ElseCherche_Numero_Fichier: ' Chercher la ligne ou le numero de fichier est egale a NumFichier. While ActiveCell.Value <> Num_Fichier If ActiveCell.Row = Range("Etiquettes").Row Then GoTo Inserer_Ligne End If ActiveCell.Offset(-1, 0).Range("a1:a1").Select Wend 'Verifier le numero d'indice de la ligne active. If Cells(ActiveCell.Row, 165).Value <> ClasseurEnCours.Sheets("DATA").Range("Dernier_Indice") Then ActiveCell.Offset(-1, 0).Range("A1:A1").Select GoTo Cherche_Numero_Fichier End If ActiveCell.Offset(0, 0).Range("A1:FM1").Value = "" End If

We start with another label, and... then we have a Goto. A Goto which jumps us back into the If side of the conditional. A Goto inside of a while loop, a while loop that's marching around the spreadsheet to search for certain values in the cell.

After the loop, we have another Goto which will possibly jump us up to the start of the else block.

The procedure ends with some cleanup:

'----- ' Do some stuff on the active cell and the following cells on the column.-----BaseDonnees.Close TrueSet BaseDonnees = NothingEnd Sub

I do not know what this function does, and the fact that the code is largely in a language I don't speak isn't the obstacle. I have no idea what the loops and the gotos are trying to do. I'm not even a "never use Goto ever ever ever" person; in a language like VBA, it's sometimes the best way to handle errors. But this bizarre time-traveling flow control boggles me.

"Etiquettes" is French for "labels", and it may be bad etiquette but I've got some four letter labels for this code.

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.