I wanted to start a short tips series for improving code and application quality, each post will be 1 to few sentences with a tip.
First tip:
Say “Defects”, not “Bugs”
For me, the word “bug” sounds as “something not so important that is in the nature of the application”, which at least it isn’t true! when the application doesn’t do what it should (or do it wrong, or crash etc) it is a Defect in our application. It is hard because we are so naturally says it, but we should start to change our terminology for better software!
According to the first principle of S.O.L.I.D, SRP – Single Responsibility Principle, a class should have one responsibility, shortly, it should have only one reason to change.
Example
Let assume that we have to create a software for importing CSV data to the database.
“Fast & Dirty”
Reading directly from the CSV file into the database, line by line, on each line parsing (in separated method, or worse inline) the string and writing it to database.
1: public class CSVtoDbImporter
2: {
3: public void Import(string fileName)
4: {
5: using (var reader = File.OpenRead(fileName))
6: {
7: string line;
8: while ((line = reader.ReadLine()) != null)
9: {
10: InsertToDb(line);
11: }
12: }
13: }
14:
15: public void InsertToDb(string line)
16: {
17: var values = line.Split(',');
18: var insertStatement = string.Format("INSERT INTO Table VALUES({0},'{1}')",values);
19: ExecuteCommand(insertStatement);
20: }
21: }
The problem here is that when the file format changes or the db schema changes this class should be updated (or rewritten), violating the SRP.
“SRP Applied”
Reading each row to corresponding POCO object that represents the row loosely from the format (doesn’t care if it CSV, XML or any other form), and a specialized reader, CSVReader that sends this object to another class, DbImporter, that knows how to import it to the db. (performance issues are not relevant here, because on (very-)large files you can still stream the object one at a time and keep the separation)
1: public class TableRow
2: {
3: public int Id { get; set; }
4: public string Name { get; set; }
5: }
6:
7: public class CSVReader : IReader
8: {
9: public TableRow[] Read(string fileName)
10: {
11: // read and parse each line into TableRow objects and return the array...
12: }
13: }
14:
15: public class DbImporter : IDbImporter
16: {
17: public void Import(TableRow[] rows)
18: {
19: // insert each row into db
20: }
21: }
In this snippet, when the database schema changed, only the DbImporter changes, and the reader remains the same. On the other side, if the file format is changed, only the reader implementation is changed.
SRP in Variables
It may seem straignt forward but this principle is violated in variables too in the form of ambiguity: using the same variable to represent more than one meaning, for example:
1: int status = (int)Statuses.SomeStatus
2:
3: // …
4:
5: if (status == (int)Statuses.SomeStatus) status = 125 /* Some other code, not a status */
6:
7: First, the principle:
Variable should have one and only one meaning – if it represents status, it is a status variable, if it represents code it is code variable, never the same.
This ambiguity WILL catch you, and it will be in the most inappropriate time. It introduce parallel flow (do not confuse with parallel computing) for the application making it very hard to maintain the code, harm the productivity and introduce defects (bugs) that are very hard to spot and fix.
Of-course, this principle applies to properties, constants, enums etc.
So, express yourself!