Monday, July 31, 2017

Two examples of Connascence of Position

This post appeared originally on Codesai’s Blog.

As we saw in our previous post about connascence, Connascence of Position (CoP) happens when multiple components must be adjacent or appear in a particular order. CoP is the strongest form of static connascence, as shown in the following figure.
Connascence forms sorted by descending strength (from Kevin Rutherford's XP Surgery).
A typical example of CoP appears when we use positional parameters in a method signature because any change in the order of the parameters will force to change all the clients using the method.

def load_session(timestamp, db, user_id)
# doing sth important
end
The degree of the CoP increases with the number of parameters, being zero when we have only one parameter. This is closely related with the Long Parameters List smell.
In some languages, such as Ruby, Clojure, C#, Python, etc, this can be refactored by introducing named parameters (see Introduce Named Parameter refactoring)[1].

# Initial method with positional parameters -> CoP
def load_session(timestamp, db, user_id)
# doing sth important
end
# A call somewhere in the code
load_session 1499853283657, db, 5
#-------------------------------------------------------------
# After applying Introduce Named Parameter refactoring -> CoN
def load_session(timestamp:, db:, user_id:)
# doing sth important
end
# A call somewhere in the code
load_session user_id: 5, db: db, timestamp: 1499853283657
Now changing the order of parameters in the signature of the method won’t force the calls to the method to change, but changing the name of the parameters will. This means that the resulting method no longer presents CoP. Instead, now it presents Connascence of Name, (CoN), which is the weakest form of static connascence, so this refactoring has reduced the overall connascence.

The benefits don’t end there. If we have a look at the calls before and after the refactoring, we can see how the call after introducing named parameters communicates the intent of each parameter much better. Does this mean that we should use named parameters everywhere?

Well, it depends. There’re some trade-offs to consider. Positional parameters produce shorter calls. Using named parameters gives us better code clarity and maintainability than positional parameters, but we lose terseness[2]. On the other hand, when the number of parameters is small, a well chosen method name can make the intent of the positional arguments easy to guess and thus make the use of named parameters redundant.

We should also consider the impact that the degree and locality of each instance of CoP[3] can have on the maintainability and communication of intent of each option. On one hand, the impact on maintainability of using positional parameters is higher for public methods than for private methods (even higher for published public methods)[4]. On the other hand, a similar reasoning might be made about the intent of positional parameters: the positional parameters of a private method in a cohesive class might be much easier to understand than the parameters of a public method of a class a client is using, because in the former case we have much more context to help us understand.

The communication of positional parameters can be improved a lot with the parameter name hinting feature provided by IDEs like IntelliJ. In any case, even though they look like named parameters, they still are positional parameters and have CoP. In this sense, parameter name hinting might end up having a bad effect in your code by reducing the pain of having long parameter lists.

Finally, moving to named parameters can increase the difficulty of applying the most frequent refactoring: renaming. Most IDEs are great renaming positional parameters, but not all are so good renaming named parameters.

A second example.

There are also cases in which blindly using named parameters can make things worse. See the following example:

def activate_alarm(lower_threshold, higher_threshold, sensor)
#doing sth important
end
activate_alarm Pressure.in_bars(3), Pressure.in_bars(15), sensor
The activate_alarm method presents CoP, so let’s introduce named parameters as in the previous example:

# Initial method with positional parameters -> CoP
def activate_alarm(lower_threshold, higher_threshold, sensor)
#doing sth important
end
# A call somewhere in the code
activate_alarm Pressure.in_bars(3), Pressure.in_bars(15), sensor
#-------------------------------------------------------------
# After applying Introduce Named Parameter refactoring -> only CoN right???
def activate_alarm(lower_threshold:, higher_threshold:, sensor:)
#doing sth important
puts lower_threshold
puts higher_threshold
puts sensor
end
# A call somewhere in the code
activate_alarm(
higher_threshold: Pressure.in_bars(15),
lower_threshold: Pressure.in_bars(3),
sensor: sensor
)
We have eliminated the CoP and now there’s only CoN, right?

In this particular case, the answer would be no. We’re just masking the real problem which was a Connascence of Meaning (CoM) (a.k.a. Connascence of Convention). CoM happens when multiple components must agree on the meaning of specific values[5]. CoM is telling us that there might be a missing concept or abstraction in our domain. The fact that the lower_threshold and higher_threshold only make sense when they go together, (we’re facing a case of data clump), is an implicit meaning or convention on which different methods sharing those parameters must agree, therefore there’s CoM.

We can eliminate the CoM by introducing a new class, Range, to wrap the data clump and reify the missing concept in our domain reducing the CoM to Connascence of Type (CoT)[6]. This refactoring plus the introduction of named parameters leaves with the following code:

# Type that reifies the missing concept Range to eliminate the CoM
class Range
attr_reader :lower_threshold, :higher_threshold
def initialize(lower_threshold:, higher_threshold:)
@lower_threshold = lower_threshold
@higher_threshold = higher_threshold
end
#some important functionality will be attracted here :)
end
# After removing the CoM (data clump in this case) -> CoT
def activate_alarm(safety_range:, sensor:)
#doing sth important
end
# being used somewhere in the code
safety_range = Range.new(higher_threshold: Pressure.in_bars(15), lower_threshold: Pressure.in_bars(3))
activate_alarm sensor: sensor, safety_range: safety_range
This refactoring is way better than only introducing named parameters because it does not only provides a bigger coupling reduction by going down in the scale from from CoP to CoT instead of only from CoP to CoM, but also it introduces more semantics by adding a missing concept (the Range object).

Later we’ll probably detect similarities[7] in the way some functions that receives the new concept are using it and reduce it by moving that behavior into the new concept converting it in a value object. It’s in this sense that we say that value objects attract behavior.

Summary.

We have presented two examples of CoP, a “pure” one and another one that was really hiding a case of CoM. We have related CoP and CoM with known code smells, (Long Parameters List, Data Clump and Primitive Obsession), and introduced refactorings to reduce their coupling and improve their communication of intent. We have also discussed a bit, about when and what we need to consider before applying these refactorings.

References.

Talks.

Books.

Posts.

Footnotes.

:
[1] For languages that don't allow named parameters, see the Introduce Parameter Object refactoring.
[3] See our previous post About Connascence.
[4] For instance, Sandi Metz recommends in her POODR book to "use hashes for initialization arguments" in constructors (this was the way of having named parameters before Ruby 2.0 introduced keyword arguments).
[5] Data Clump and Primitive Obsession smells are examples of CoM.
[6] Connascence of Type, (CoT), happens when multiple components must agree on the type of an entity.
[7] Those similarities in the use of the new concept are examples of Conascence of Algorithm which happens when multiple components must agree on a particular algorithm.

Sunday, July 2, 2017

Kata: LegacySecurityManager in Java

This week I did the LegacySecurityManager kata in Java.

This is the original version of the code (ported from C# to Java):
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
public class SecurityManager {
public static void createUser() {
BufferedReader buffer = new BufferedReader(new InputStreamReader(System.in));
String username = null;
String fullName = null;
String password = null;
String confirmPassword = null;
try {
System.out.println("Enter a username");
username = buffer.readLine();
System.out.println("Enter your full name");
fullName = buffer.readLine();
System.out.println("Enter your password");
password = buffer.readLine();
System.out.println("Re-enter your password");
confirmPassword = buffer.readLine();
} catch (IOException e) {
e.printStackTrace();
}
if (!password.equals(confirmPassword)) {
System.out.println("The passwords don't match");
return;
}
if (password.length() < 8) {
System.out.println("Password must be at least 8 characters in length");
return;
}
// Encrypt the password (just reverse it, should be secure)
String encryptedPassword = new StringBuilder(password).reverse().toString();
System.out.println(
String.format(
"Saving Details for User (%s, %s, %s)\n",
username,
fullName,
encryptedPassword)
);
}
}
As you can see, createUser is a really hard to test static function which has too many responsibilities.

This is the final version of createUser after a "bit" of refactoring:
import user_creation.*;
public class SecurityManager {
public static void createUser() {
Console console = new RealConsole();
new CreatingUser(
new ConsoleUserDataRetrieval(console),
new ConsoleReporter(console),
new ReverseEncryption()
).execute();
}
}
which is using the CreatingUser class:
package user_creation;
public class CreatingUser {
private static final int MINIMUM_PASSWORD_LENGTH = 8;
private static final String NOT_MATCHING_PASSWORDS_ERROR = "The passwords don't match";
private static final String TOO_SHORT_PASSWORD_ERROR = "Password must be at least 8 characters in length";
private final EncryptionAlgorithm encryptionAlgorithm;
private UserDataRetrieval userDataRetrieval;
private Reporter reporter;
public CreatingUser(
UserDataRetrieval userDataRetrieval,
Reporter reporter,
EncryptionAlgorithm encryptionAlgorithm
) {
this.userDataRetrieval = userDataRetrieval;
this.reporter = reporter;
this.encryptionAlgorithm = encryptionAlgorithm;
}
public void execute() {
UserData userData = userDataRetrieval.invoke();
if (!userData.passwordsMatch()) {
reporter.reportError(NOT_MATCHING_PASSWORDS_ERROR);
return;
}
if (isPasswordTooShort(userData)) {
reporter.reportError(TOO_SHORT_PASSWORD_ERROR);
return;
}
String encryptedPassword = encryptionAlgorithm.encrypt(userData.password());
reporter.reportSuccessCreatingUser(
composeMessage(userData, encryptedPassword)
);
}
private boolean isPasswordTooShort(UserData userData) {
return userData.passwordLength() < MINIMUM_PASSWORD_LENGTH;
}
private String composeMessage(UserData userData, String encryptedPassword) {
return new UserCreationSuccessMessage(userData, encryptedPassword).compose();
}
}
The before and after code is not so interesting as how we got there.

I kept the legacy code interface and tried to use as much as possible only automatic refactorings (mainly Replace Method with Method Object and Extract Method) to apply the extract and override dependency-breaking technique, from Michael Feather's Working Effectively with Legacy Code book, which enabled me to write tests for the code.

Then, with the tests in place, it was a matter of identifying and separating responsibilities and introducing some value objects. This separation allowed us to remove the scaffolding produced by the extract and override technique producing much simpler and easier to understand tests.

You can follow the process seeing all the commits (I committed changes after every refactoring step). There you'll be able to see there not only the process but my hesitations, mistakes and changes of mind as I learn more about the code during the process.

You can also find all the code on GitHub.

After reflecting on what I did I realized that I could have done less to get the same results by avoiding some tests that I later found out where redundant and deleted. I also need to improve my knowledge of IntelliJ automatic refactorings to improve my execution (that part you can't see in the commits).

All in all is a great kata to practice your refactoring skills.

Interesting Talk: "Functional architecture. The pits of success"

I've just watched this great talk by Mark Seemann