Created
February 20, 2019 08:31
-
-
Save sergioverde90/4525fae6e670213cad57a051fb30b2a2 to your computer and use it in GitHub Desktop.
Sergio Verde - Refactor example
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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()); | |
} | |
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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