Implement simplified liquibase

Question

I attended job interview and on live codeing session was asked to implement code based on some requirements. There are quite long text description but idea that we have to accept some file system path and run some files if they satisfy some name pattern and depends on pattern SqlRunner should call different methos. I am free to create interfaces or can use any existing java API.

I've created following code:

class MigrationProcessor {
 private static final String[] STOP_FOLDER_NAMES = new String[]{"tmp", "system"};
 private SqlRunner sqlRunner;
 private FileSystemReader fileSystemReader;
 public void process(Path path) {
 for (FileSystemSObject fileSystemSObject : fileSystemReader.getAllObjects(path)) {
 if (fileSystemSObject.isFile()) {
 processFile(fileSystemSObject);
 } else if (fileSystemSObject.isFolder()) {
 processFolder(fileSystemSObject);
 }
 }
 }
 private void processFolder(FileSystemSObject fileSystemSObject) {
 if (Arrays.asList(STOP_FOLDER_NAMES).contains(fileSystemSObject.getName())) {
 System.out.println("skipped processing of folder " + fileSystemSObject.getName());
 } else {
 process(fileSystemSObject.getPath());
 }
 }
 private void processFile(FileSystemSObject fileSystemSObject) {
 String fileName = fileSystemSObject.getName();
 ScriptType scriptType = ScriptType.getTypeByName(fileName);
 if (scriptType == null) {
 System.out.println("Ignore file " + fileSystemSObject.getName());
 } else {
 switch (scriptType) {
 case SQL:
 sqlRunner.runSql(fileName);
 break;
 case CREATE_INDEX:
 sqlRunner.createIndex(fileName);
 break;
 case RUN_STORED_PROCEDURE:
 sqlRunner.runStoredProcedure(fileName);
 break;
 }
 }
 }
}
interface SqlRunner {
 void runSql(String fileName);
 void createIndex(String fileName);
 void runStoredProcedure(String fileName);
}
interface FileSystemReader {
 Iterable<FileSystemSObject> getAllObjects(Path path);
}
interface FileSystemSObject {
 boolean isFolder();
 boolean isFile();
 String getName();
 Path getPath();
}
enum ScriptType {
 SQL,
 CREATE_INDEX,
 RUN_STORED_PROCEDURE;
 public static ScriptType getTypeByName(String name) {
 if (name.startsWith("create_index_") && name.endsWith(".sql")) {
 return CREATE_INDEX;
 } else if (name.startsWith("run_procedure_") && name.endsWith(".sql")) {
 return RUN_STORED_PROCEDURE;
 } else if (name.endsWith(".sql")) {
 return SQL;
 }
 return null;
 }
}

please review

Answer 1

Some thoughts:

STOP_FOLDER_NAMES should be a Set for O(1) lookups. It should definitely not get converted from an array into a list every time processFolder runs.

STOP_FOLDER_NAMES might be better named "FOLDERS_TO_IGNORE". As I’m old school, I also prefer the term "directory", but that’s probably just me.

It’s not clear how one starts MigrationProcessor off. If you start from a directory which is below one of the folder names to ignore, the code should ignore all of them but will instead process all of them.

What does the S in FileSystemSObject` stand for? Script? The extra characters are free and add to readability.

FileSystemScriptObject appears to be a subset of the functionality available in File. Since you’re already using Path, is there a reason you don’t also use File?

processFile would be cleaner if the unknown ScriptType was handled in a default case, rather than checking it earlier.

How can processFile’s sqlRunner locate the file with only the filename? Isn’t the full path required?

Writing to System.out is suboptimal. Use a logger. At the very least, errors should go to System.err.

It’s unclear how large the file system is, but you could theoretically run into issues if you have to recurse too deeply. Hopefully you asked the interviewer about that.

It would be nice if there was javadoc on FileSystemScriptObject explaining the difference between "name" and "path". Is "name" just a simple name, or does it have the whole directory structure in String form?

ScriptType.getTypeByName() might read better as ScriptType.forFilename()

getTypeByName may wish to accept a FileSystemSObject or a path instead of a filename. It seems reasonable that script type detection may rely on the contents of a file.

There’s no allowance for failure when SqlRunner executes.

A big advantage of Liquibase is being able to roll back files. This application doesn’t support that. Unclear if that was part of the problem statement.