Gör det ont, så gör jag fel

Squeeds Julkalender | 2019-12-01 | Stefan Cronert
Självinsikt är av stor vikt för en utvecklare; att vet vad du förstår och förstå vad du vet, hjälper dig att hitta rätt i alla val du kan göra som. Istället för att tro att det är fel på det ramverk eller kodbibliotek som du använder dig av, så kan det vara bra att ta ett steg tillbaka och titta på din kod. Som ett konkret exempel på mina egna irrvägar så har jag tagit med kod från ett verkligt fall (dock är den ändrad för att skydda oskyldiga)
code_despair.jpg

Irrvägar

Tidigare i min utvecklarkarriär, så var jag ganska kvick på att döma ut ramverk och bibliotek, när det inte fungerar så enkelt och smärtfritt som jag trodde att de skulle göra. Jag låg ganska långt till vänster på Dunning-Kruger kurvan, dvs ju mer jag vet desto mindre förstår jag (egen tolkning) och därför trodde jag att jag hade koll på läget och omöjligen kunde göra fel.

Att veta hur lite jag förstår och förstå hur lite jag vet, har hjälpt mig att sluta stånga mig blodig mot mina egna felaktiga antaganden och istället ta ett steg tillbaka och fundera på vad jag vet och förstår.

Låt mig ta ett exempel, jag använde Dapper för att hämta data från en databas och göra om det till objekt i koden. Efter lite knåpande så fick jag till följande kod.

public IEnumerable<ValueObject.Person> List()
{
  using (var connection = _connectionFactory.Create("Employees"))
  {
    connection.Open();
    var query = "SELECT * FROM Persons";
    var persons = connection
                  .Query(query, parameters)
                  .Select(Convert);
    return persons;
  }
}

private static ValueObject.Person Convert(dynamic p)
{
  IDictionary<string, object> row = (IDictionary<string, object>)p;
  var number = (string)row.First(kvp => kvp.Key == "employeeNumber").Value;
  var employeeNumber = string.IsNullOrEmpty(number)
                     ? null
                     : new Common.ValueObjects.EmployeeNumber(int.Parse(number));
  return new Person {
    Id = p.Id,
    FirstName = p.FirstName,
    Lastname = p.LastName,
    EmployeeNumber = employeeNumber
  };
}

Den fungerade alldeles utmärkt när jag körde den mot det testdata jag hade tillgängligt och på de enhetstester som jag inte hade skrivit. 

Givetvis visade det sig att det var en bugg i Convert-metoden och ni som är bättre utvecklare än jag ser snabbt var felet låg.

Nåväl som en god utvecklare så tänkte jag att jag skriver ett enhetstest för att visa att koden i dess dåvarande form är fel och efter rättning att den är rätt.

Det visade sig vara lite svårare än jag trodde, eftersom Query ovan genererar en IEnumerable<Dapper.SqlMapper.DapperRow>, men eftersom DapperRow är både private och sealed (och därmed inte ska användas av min kod) men däremot är det som returneras dynamic och kan konverteras till IDictionary<string, string> då den ärver av Dictionary<string, string>. 

Det hade kunnat fungera, om ett fält i det hämtade datat som är null hade blivit en post med med kolumnnamnet som nyckel och värdet null. Istället så skapas det ingen post i IDictionary<string,string> (det var något jag kom på efter att jag tror mig hittat en mer korrekt lösning, se nedan) över huvud taget. Om det är fallet så kommer row.First att kasta ett exception om det värdet på kolumnen som jag jämför med är null (då finns ju inte nyckeln)

Det testdatat som jag hade tillgång till så hade alla poster EmployeeNumber, men så var inte fallet i test-miljön som hade mer produktionslik data. 

Att sätta upp tester och fixa med att komma åt den privata stängda klassen DapperRow gjorde ont och tog tid, och som tur var kom jag på det innan jag lagt ner alltför mycket tid och energi att lösa ett felaktigt problem.

Men jag började tveka om Dapper verkligen var det ramverk som uppfyllde de krav som jag hade, tills jag insåg att mitt sätt att använda Dapper var fel och tog ett steg tillbaka och gjorde om.

Vägen tillbaka

Vad är den mer (enligt mig) korrekta lösningen? Det är att ha en dataklass (Dto.Person) som används enbart för att stoppa in datat från databasen och sedan konvertera till en klass i affärslogiken.

public IEnumerable<ValueObject.Person> List()
{
  using ( var connection = _connectionFactory.Create("Employees"))
  {
    connection.Open();
    var query = "SELECT * FROM Persons";
    var persons = connection
                  .Query<Dto.PersonInfo> (query, parameters)
                  .Select(Convert);
    return persons;
  }
}

internal static ValueObjects.Person Convert(Dto.PersonInfo p)
{
  var number = p.EmployeeNumber.HasValue
             ? new Common.ValueObjects.EmployeeNumber(p.EmployeeNumber.Value)
             : null;
  return new ValueObjects.Person {
     Id = p.Id,
     FirstName = p.FirstName,
     Lastname = p.LastName,
     EmployeeNumber = employeeNumber
  };
}

Take aways

Den som har utvecklat biblioteket har lagt ner många timmar för att lösa ett specifikt problem, vilket det förhoppningsvis gör och du har kanske är samma problem som du har.

Använder du ett bibliotek är det för att lösa ett problem och fungerar inte biblioteket för din kod så har du kanske inte det problemet eller så skapar din kod problem som inte biblioteket löser (vilket var fallet för mig).