Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Sergio Verde - Refactor example
package untestableLegacy;
/*
* This code has been extracted from OpenbravoERP:
* https://code.openbravo.com/erp/stable/2.50-bp/file/cec9b1da72ed/src/org/openbravo/base/model/Entity.java
*/
import java.nio.CharBuffer;
import java.util.List;
import java.util.stream.Collectors;
class Log{
public void warn(String msg){
System.out.println(msg);
}
}
public class Entity {
private static final char[] ILLEGAL_ENTITY_NAME_CHARS = new char[]{'*', '?'};
private String name;
private Log log;
public Entity(Log log) {
this.log = log;
}
String getName() {
return name;
}
// SMELL-1: The name is not revealing the real behaviour
void setNameRemovingIllegalChars(String name) {
// SMELL-2: Use intention naming instead comments
this.name = removeIllegalChars(name);
}
private String removeIllegalChars(String originalName) {
final char[] chars = originalName.toCharArray();
List<Character> characters = arrayToList(chars);
List<Character> illegal = arrayToList(ILLEGAL_ENTITY_NAME_CHARS);
List<Character> onlyLegalCharacters = removeIllegalCharacters(characters, illegal);
String newName = charactersToString(onlyLegalCharacters);
logIfNameHasChanged(originalName, newName);
logIfContainsNonNormalChars(originalName);
return newName;
}
private void logIfNameHasChanged(String originalName, String newName) {
boolean nameChanged = !newName.equals(originalName);
if (nameChanged) {
log.warn("The entity name " + originalName
+ " contains illegal characters, it has been repaired to " + newName);
}
}
private void logIfContainsNonNormalChars(String originalName) {
// check for other less normal characters
for (char c : originalName.trim().toCharArray()) {
if (!isNormalChar(c)) {
log.warn("The entity name " + originalName + " contains a character (" + c
+ ") which could result in issues in HQL or "
+ "webservices. Use characters from a to z, A to Z or 0 to 9 or the _");
}
}
}
private static String charactersToString(List<Character> withoutIllegalCharacters) {
return withoutIllegalCharacters.stream().map(String::valueOf).collect(Collectors.joining());
}
private static List<Character> removeIllegalCharacters(List<Character> characters, List<Character> illegal) {
return characters.stream().filter(c -> !illegal.contains(c)).collect(Collectors.toList());
}
private static boolean isNormalChar(char c) {
return isBetween(c, 'A', 'Z') || (isBetween(c, '0', '9')) || (isBetween(c, 'a', 'z')) || c == '_';
}
private static boolean isBetween(char toTest, char from, char to) {
return from <= toTest && toTest <= to;
}
private static List<Character> arrayToList(char[] chars) {
return CharBuffer.wrap(chars).chars().mapToObj(i -> (char) i).collect(Collectors.toList());
}
}
package untestableLegacy;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public class EntityTest {
private final CustomLog log = new CustomLog(new Log());
@Test
public void shouldRemoveIllegalCharacters() {
Entity entity = new Entity(log);
entity.setNameRemovingIllegalChars("refactoring*");
assertEquals(entity.getName(), "refactoring");
}
@Test
public void shouldPreserveOriginalNameIfNoIllegalCharacters() {
Entity entity = new Entity(log);
entity.setNameRemovingIllegalChars("refactoring");
assertEquals(entity.getName(), "refactoring");
}
@Test
public void shouldNotLogAnythingIfNameDoesNotContainsIllegalOrNonNormalCharacters() {
Entity entity = new Entity(log);
entity.setNameRemovingIllegalChars("refactoring");
assertTrue(log.messages.isEmpty());
}
@Test
public void shouldLogIfNonNormalChar() {
Entity entity = new Entity(log);
entity.setNameRemovingIllegalChars("refactoring%");
assertTrue(log.containsPartially("Use characters from a to z, A to Z or 0 to 9 or the _"));
}
static class CustomLog extends Log {
List<String> messages = new ArrayList<>();
Log log;
CustomLog(Log log) {
this.log = log;
}
@Override
public void warn(String msg) {
messages.add(msg);
log.warn(msg);
}
boolean containsPartially(String msg) {
return messages.stream().anyMatch(s -> s.contains(msg));
}
}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.