Clean Code is Not Threadsafe Code?
I am currently reading Clean Code: A Handbook of Agile Software Craftsmanship by Robert C. Martin and I’ve noticed a pattern.
In Listing 2-1, Martin recommends refactoring this code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
private void printGuessStatistics (char candidate, int count)
{
String number;
String verb;
String pluralModifier;
if (count == 0) {
number = "no" ;
verb = "are" ;
pluralModifier = "s" ;
} else if (count == 1) {
number = "1" ;
verb = "is" ;
pluralModifier = "" ;
} else {
number = Integer.toString (count);
verb = "are" ;
pluralModifier = "s" ;
}
String guessMessage = String.format (
"There %s %s %s%s" , verb, number, candidate, pluralModifier
);
print(guessMessage);
}
into this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
public class GuessStatisticsMessage {
private String number;
private String verb;
private String pluralModifier;
public String make (char candidate, int count) {
createPluralDependentMessageParts(count);
return String.format (
"There %s %s %s%s" ,
verb, number, candidate, pluralModifier);
}
private void createPluralDependentMessageParts (int count) {
if (count == 0) {
thereAreNoLetters();
} else if (count == 1) {
thereIsOneLetter();
} else {
thereAreManyLetterscount();
}
}
private void thereAreManyLetters (int count) {
number = Integer.toString (count);
verb = "are" ;
pluralModifier = "s" ;
}
private void thereIsOneLetter () {
number = "1" ;
verb = "is" ;
pluralModifier = "" ;
}
private void thereAreNoLetters () {
number = "no" ;
verb = "are" ;
pluralModifier = "s" ;
}
}
In listing 4-7, Martin recommends refactoring this code (comments elided):
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
public class GeneratePrimes
{
public static int [] generatePrimes (int maxValue)
{
if (maxValue >= 2) {
int s = maxValue + 1;
boolean f = new boolean [ s] ;
int i;
for (i = 0; i < s; i++ )
f[ i] = true ;
f[ 0] = f[ 1] = false ;
int j;
for (i = 2; i < Math.sqrt (s) + 1; i++ ) {
if (f[ i] ) {
for (j = 2 * i; j < s; j += i)
f[ j] = false ;
}
}
int count = 0;
for (i = 0; i < s; i++ ) {
if (f[ i] ) {
count++ ;
}
}
int [] primes = new int [ count] ;
for (i = 0, j = 0; i < s; i++ ) {
if (f[ i] )
primes[ j++] = i;
}
return primes;
} else {
return new int [ 0] ;
}
}
}
into this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
public class PrimeGenerator
{
public static boolean [] crossedOut;
public static int [] result;
public static int [] generatePrimes (int maxValue)
{
if (maxValue < 2) {
return new int [ 0] ;
} else {
uncrossIntegersUpTo(maxValue);
crossOutMultiples();
putUncrossedIntegersIntoResult();
return result;
}
}
private static void uncrossIntegersUpTo (int maxValue)
{
crossedOut = new boolean [ maxValue + 1] ;
for (int i = 2; i < crossedOut.length ; i++ )
crossedOut[ i] = false ;
}
private static void crossOutMultiples ()
{
int limit = determineIterationLimit();
for (int i = 2; i <= limit; i++ ) {
if (notCrossed(i))
crossOutMultiplesOf(i);
}
}
private static int determineIterationLimit ()
{
double iterationLimit = Math.sqrt (crossedOut.length );
return (int )iterationLimit;
}
private static void crossOutMultiplesOf (int i)
{
for (int multiple = 2* i; multiple < crossedOut.length ; multiple += i)
crossedOut[ multiple] = true ;
}
private static boolean notCrossed (int i)
{
return crossedOut[ i] == false ;
}
private static void putUncrossedIntegersIntoResult ()
{
result = new int [ numberOfUncrossedIntegers()] ;
for (int j = 0, i = 2; i < crossedOut.length ; i++ ) {
if (notCrossed(i))
result[ j++] = i;
}
}
private static int numberOfUncrossedIntegers ()
{
int count = 0;
for (int i = 2; i < crossedOut.length ; i++ )
if (notCrossed(i))
count++ ;
return count;
}
}
In both cases, Martin has transformed thread-safe , nearly pure functions into thread-unsafe functions. Even worse, while the former example can be used on multiple threads by having each thread instantiate a local instance of GuessStatisticsMessage
, the latter example, due to its use of static member variables, cannot be used on multiple threads without some form of locking.
While I agree that Martin’s examples are more readable, it seems to me that there may be a fundamental tension between object-oriented programming and thread safety, and this tension does not exist in functional programming .