Sergio Verde - Refactor example

Entity.java

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());
	}

}

EntityTest.java

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));
        }
    }

}