Tag: structure extraction

A friend once told me that when his workday goes sour, he turns to a practice of “therapeutic refactoring”: Making little improvements throughout the source code of a project without an overarching goal. If we dissect this behaviour, we probably find a dopamine-based reward cycle not unfamiliar to those in popular video games like Diablo, only that we slay little flaws in the source code instead of critters. And because we never know what code can be found in the next method and the next class (just like we don’t know which monsters lurk behind the next door in Diablo), we go and explore for a bit, leaving a trail of resolved code smells (or slain monsters) behind.

But somewhere in each level of Diablo, there waits a boss enemy that requires some more hammering. And similar, every now and then you encounter a code area that looks dirty but is not resolved that easily. This blog entry tells the story of one of these encounters and how we refactored it. There is no guarantee that you’ll find the result “better” in any way, the whole point is that it can be done and the mechanic is always the same.

Let’s look at this code snippet, reduced to the relevant parts:

public static void setValueFor(
    final PreparedStatement statement,
    final ColumnInfo column,
    final String value
) throws SQLException {
    final int typeNumber = column.type().getVendorTypeNumber();
    switch (typeNumber) {
        case Types.DATE -> {
            statement.setDate(
                column.columnIndex(),
                new Date(
                    getDateMillisFrom(
                        LocalDate.from(parseToDate(value)).atStartOfDay(ZoneOffset.UTC)
                    )
                )
            );
        }
        case Types.TIMESTAMP -> {
            statement.setTimestamp(
                column.columnIndex(),
                new Timestamp(
                    getDateMillisFrom(
                        LocalDateTime.from(parseToDateTime(value)).atZone(ZoneOffset.UTC)
                    )
                )
            );
        }
    }
}

A given string value is written into a SQL database, using a prepared statement and adequatly typed setter method calls. The real code is way more exhaustive, but we are only interested in the striking similarity of the DATE and TIMESTAMP case.

The structure resembles something I call a “sandwich” or “burger”: The same bun above and below, but different things in the middle. If it is only “bun, patty, bun”, you can solve it with a simple lambda expression as a method parameter. But in this case, it is a more delicate mix of bread and filling layers. Let’s investigate to plan our refactoring attempt:

• statement.setDate() and statement.setTimestamp(): Both operate on the prepared statement, but call different methods with a different parameter type. Needs to be configurable in the result.
• column.columnIndex(): Duplication. Needs to be part of the extracted code.
• new Date() and new Timestamp(): There is no suitable supertype other than java.util.Date. Since we want the actual parameter types for the final setter call, we shouldn’t abstract to it. We need to make this call configurable, but be able to work with the result without knowing its actual type. A generic parameter is the right choice for it.
• getDateMillisFrom(): Duplication. Needs to be part of the extracted code.
• LocalDate.from() and LocalDateTime.from(): They look similar enough to be tempted to unify them into one generic call, but it would refrain from doing it. This feels like a pyrrhic victory trap to me, where the solution is way more complex than the original. I already fear this effect for our initial quest, I don’t need a nested sub-quest right now.
• .atStartOfDay(ZoneOffset.UTC) and .atZone(ZoneOffset.UTC): Duplication. Needs to be part of the extracted code.

If you don’t see my classification right away, my suggestion is to print the code snippet on paper, grabbing a green and a blue text marker and separating each term by coloring it. I did this for years until I could color the code right in place in my imagination. And with training, I could add more colors…

So we have a structure like: Configure, Extract, Configure, Extract, Configure, Extract. Nice! I’ve counted three configuration parameters and three entities that have their “single source of truth” in the extracted code. We also have six steps in the total story. Let’s plan it:

private static void setTimetype(
    ColumnInfo column,
	String value,
    

Ok, this is the foundation. We need the information about the column and the value to work on both. Now on to the configurable parts. I start with the innermost parameter: Something that converts the String value into a LocalDateTime (the time zone aspect is part of our extracted code):

private static void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
    

That was the easy part. For the next configurable part, we need to be able to support both Date and Timestamp. Both are created from a long, representing milliseconds since epoch start. This is only easy if we make it easy in the moment:

private static void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
	Function<Long, T> creation,
    

We added a generic type “T” that we need to specify so the compiler doesn’t act up:

private static <T> void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
	Function<Long, T> creation,
    

We specify the generic in the context of the method. Not giving any clue about what types are valid keeps us maneuverable in this dimension. There is one improvement I want to make that is unusual in Java: Any name for a generic type is always one capital letter in all examples. There is no need to be so spartanic with letters. We can just name our “T” like any other type. I like “Timetype” (yes, with lowercase letters, get over it) because the method name talks about a “Timetype” already:

private static <Timetype> void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
	Function<Long, Timetype> creation,
    

If you are unfamiliar with this naming scheme: The type “Timetype” doesn’t actually exist, just like “T” didn’t exist. I just view generic type names as “normal” enough to give them normal names and not just “T”, “X”, “A”, etc.

That was our second configurable parameter and it forced us to make nearly all important decisions. The third one follows suit:

private static <Timetype> void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
	Function<Long, Timetype> creation,
	Consumer<Integer, Timetype> persistence
)

The outmost calls of both code lines in the original take an integer and the specific newly created type and stores it in the prepared statement. By making everything type specific a parameter, we push all the knowledge about the prepared statement outside our extracted method. All the knowledge, with the exception of exceptions. Unfortunately, the statement.setDate() and statement.setTimestamp() both throw the checked exception SQLException. A java Consumer can’t handle checked exceptions. We need our own type for that:

public static interface TimetypePersister<Timetype> {
	void persist(
        int columnIndex,
        Timetype value
    ) throws SQLException;
}

This type replaces the Consumer in our signature:

private static <Timetype> void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
	Function<Long, Timetype> creation,
	TimetypePersister<Timetype> persistence
) throws SQLException {

And now we are ready to fill in the blanks. First we refactor both calls to our method:

case Types.DATE -> {
	setTimetype(
		column,
		value,
		t -> LocalDate.from(parseToDate(t)).atStartOfDay(),
		Date::new,
		statement::setDate
	);
}
case Types.TIMESTAMP -> {
	setTimetype(
		column,
		value,
		t -> LocalDateTime.from(parseToDateTime(t)),
		Timestamp::new,
		statement::setTimestamp
	);
}

You probably see the nested sub-quest of the LocalDateTime-parsing right away. As I said, I won’t attempt to solve it in this blog entry. The exercise is left to the inclined reader.

Now, we can finally implement our extracted method that retains the structure of the original lines and includes the three “entities of wisdom” that are now de-duplicated:

private static <Timetype> void setTimetype(
    ColumnInfo column,
	String value,
	Function<String, LocalDateTime> parsing,
	Function<Long, Timetype> creation,
	TimetypePersister<Timetype> persistence
) throws SQLException {
	LocalDateTime local = parsing.apply(value);
	ZonedDateTime zoned = local.atZone(ZoneOffset.UTC);
	long millis = getDateMillisFrom(zoned);
	Timetype settable = creation.apply(millis);
	int index = column.columnIndex();
	persistence.persist(
	    index,
		settable
	);
}

You can see how we unpacked the original lines in reverse: Our method begins with the last duplicated wisdom, the time zone. Then we turn the ZonedDateTime into the milliseconds since epoch start and finally, we use the column index from the given column. In the same tenor, the calls to the configurable parts are also unrolled from the innermost to the outside.

And there we are! The boss enemy is slain. The complex and intertwined structure is extracted, most duplications are resolved. We can enjoy our dopamine hit and call it a day.

Thank you for reading this far and thinking along. What part made you pause? Maybe I can improve my explanation there for you. Drop me a comment!