I've posted a question on codereview.stackexchange.com regarding my Kotlin code, but understandably few kotlin coders are on CodeReview. Therefore, I would like to request a code review. Tear it apart, criticize it, suggest improvements and emerging kotlin idioms that I may have missed.
Original Question:
For practice I decided that I wanted to write an animation class for SpriteSheets. The problem is that a single Sprite can have different widths depending on the action they are drawn to act. A tool called Sprite Sheet Packer creates a Map file documenting the index and bounds of each sprite. This file looks a little like this:
0 = 157 184 123 189
1 = 187 0 177 183
2 = 402 181 105 189
3 = 495 0 111 180
4 = 710 0 102 171
walking = 1 2 3 4
Note that the walking = 1 2 3 4 is actually not part of the map file, but I want to add some extra information, such as which animation is comprised of which frames. This I convert into the following yaml:
Frames:
- Index: 0
Bounds: [157, 184, 123, 189]
- Index: 1
Bounds: [187, 0, 177, 183]
- Index: 2
Bounds: [402, 181, 105, 189]
- Index: 3
Bounds: [495, 0, 111, 180]
- Index: 4
Bounds: [710, 0, 102, 171]
Animations:
- Name: walking
Frames: [1, 2, 3, 4]
This is my code:
data class TextParseException(message: String = "", cause: Throwable? = null): RuntimeException(message, cause)
/**
- <h2>Converts a Metadata .txt file into a .yaml file.</h2>
-
- <b>Format of a metadata text file:</b>
- <pre>
- <index>: <x> <y> <width> <height>
- …
-
- <name>: <frame1> <frame2> … <frameN>
- …
- </pre>
-
- <p>Be aware that a blank line (between … and <name>) is the separator between
- frame and animation entries. Trailing newspaces are ok, but any others will break
- the parsing.</p>
-
- <b>Output format:</b>
- <pre>
- Frames:
-
- Index: <index>
* Bounds: [<x>, <y>, <width>, <height>]
-
...
- Animations:
-
- Name: <name<
* Frames: [<frame1>, <frame2>, …, <frameN>
-
</pre>
-
-
@param metadata path to metadata file for parsing
-
@returns formatted .yaml string of metadata contents
-
@throws TextParseException if an error occurs parsing a frame or animation entry
-
@throws IllegalArgumentException if the metadata file does not exist
-
@throws IOException if the file could not be accessed or read
*/
fun convertToYaml(metadata: Path): String {
Preconditions.checkArgument(Files.exists(metadata),
“The file ${metadata.getFileName()} does not exist”)
val lines = Files.readAllLines(metadata, StandardCharsets.UTF_8).map { it.trim() }
if (lines.all { it.isEmpty() })
return “”
val yamlFrameEntries = gatherRawFrameEntries(lines)
.map(::split)
.map(::toFrameEntry)
.sortBy { it.index }
.map(::createYamlFrameEntry)
val yamlAnimationEntries = gatherRawAnimationEntries(lines)
.map(::split)
.map(::toAnimationEntry)
.map(::createYamlAnimationEntry)
return buildCompleteYaml(yamlFrameEntries, yamlAnimationEntries)
}
private fun gatherRawFrameEntries(lines: List<String>): List<String> {
return lines.takeWhile { it.isNotEmpty() }
}
private fun gatherRawAnimationEntries(lines: List<String>): List<String> {
return lines.dropWhile { it.isNotEmpty() }.drop(1).filter { it.isNotEmpty() }
}
private fun split(line: String): List<String> {
val normalizedEntry = line.replace(“=”, " ")
val parts = Splitter.on(’ ')!!
.trimResults()!!
.omitEmptyStrings()!!
.split(normalizedEntry)!!
return parts.toList()
}
private data class FrameEntry(val index: Int, val bounds: List<Int>)
private data class AnimationEntry(val name: String, val frames: List<Int>)
private fun toFrameEntry(entryPieces: List<String>): FrameEntry {
try {
val frameIndex = entryPieces.first!!
val frameBounds = entryPieces.drop(1).map { it.toInt() }
return FrameEntry(frameIndex.toInt(), frameBounds)
} catch (e: Exception) {
throw TextParseException(“Could not parse a Frame entry.”, e)
}
}
private fun toAnimationEntry(entryPieces: List<String>): AnimationEntry {
try {
val animationName = entryPieces.first!!
val animationFrames = entryPieces.drop(1).map { it.toInt() }
return AnimationEntry(animationName, animationFrames)
} catch (e: Exception) {
throw TextParseException(“Could not parse an Animation entry.”, e)
}
}
private fun createYamlFrameEntry(frameEntry: FrameEntry): String {
val yamlFrameEntryBuilder = StringBuilder()
yamlFrameEntryBuilder.append(" - Index: ${frameEntry.index}n")
yamlFrameEntryBuilder.append(" Bounds: [“)
yamlFrameEntryBuilder.append(”${Joiner.on(", “)!!.join(frameEntry.bounds)}]n”)
return yamlFrameEntryBuilder.toString()
}
private fun createYamlAnimationEntry(animationEntry: AnimationEntry): String {
val yamlFrameEntryBuilder = StringBuilder()
yamlFrameEntryBuilder.append(" - Name: ${animationEntry.name}n")
yamlFrameEntryBuilder.append(" Frames: [“)
yamlFrameEntryBuilder.append(”${Joiner.on(", “)!!.join(animationEntry.frames)}]n”)
return yamlFrameEntryBuilder.toString()
}
private fun buildCompleteYaml(yamlFrameEntries: List<String>,
yamlAnimationEntries: List<String>): String {
val completeYamlBuilder = StringBuilder()
appendYamlEntries(completeYamlBuilder, “Frames:”, yamlFrameEntries)
if (yamlAnimationEntries.isNotEmpty())
appendYamlEntries(completeYamlBuilder, “Animations:”, yamlAnimationEntries)
return completeYamlBuilder.toString()
}
private fun appendYamlEntries(builder: StringBuilder,
sectionName: String,
entries: List<String>) {
builder.append(“${sectionName}n”)
entries.forEach { builder.append(it) }
}
These are my tests:
class ConverterSpec: Spek() {{
given("A .txt -> .yaml converter") {
on("loading a non-existent file") {
it("throws an IllegalArgumentException") {
failsWith(javaClass<IllegalArgumentException>()) {
convertToYaml(Paths.get("Missing.txt")!!)
}
}
}
on("loading an empty file") {
val emptyFileUrl = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/Empty.txt”)!!
it(“produces empty yaml”) {
val output = convertToYaml(Paths.get(emptyFileUrl.toURI())!!)
assertTrue(output.isEmpty(),
“YAML output not empty, but: $output”)
}
}
on("loading file with on frame entry") {
val frameFileUrl = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/Frame.txt”)!!
it(“produces corresponding yaml for frame”) {
val expected = “Frames:n” +
" - Index: 0n" +
" Bounds: [157, 184, 123, 189]n"
val output = convertToYaml(Paths.get(frameFileUrl.toURI())!!)
assertEquals(expected, output,
“The YAML did not conform to the expected output.”)
}
}
on("loading file with multiple frame entries") {
val framesFileUrl = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/Frames.txt”)!!
it(“produces corresponding yaml for all frames”) {
val expected = “Frames:n” +
" - Index: 0n" +
" Bounds: [157, 184, 123, 189]n" +
" - Index: 1n" +
" Bounds: [187, 0, 177, 183]n" +
" - Index: 2n" +
" Bounds: [402, 181, 105, 189]n"
val output = convertToYaml(Paths.get(framesFileUrl.toURI())!!)
assertEquals(expected, output,
“The YAML did not conform to the expected output.”)
}
}
on("loading file with frames and animations") {
val frameAnimationFileUrl = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/FrameAnimation.txt”)!!
it(“produces corresponding yaml incorporating an animation name and number of frames”) {
val expected = “Frames:n” +
" - Index: 0n" +
" Bounds: [157, 184, 123, 189]n" +
" - Index: 1n" +
" Bounds: [187, 0, 177, 183]n" +
" - Index: 2n" +
" Bounds: [402, 181, 105, 189]n" +
“Animations:n” +
" - Name: Walkingn" +
" Frames: [0, 1, 2]n"
val output = convertToYaml(Paths.get(frameAnimationFileUrl.toURI())!!)
assertEquals(expected, output,
“The YAML did not conform to the expected output.nPerhaps the Animations section is malformed?”)
}
}
on("loading file with no frames but animations") {
val onlyAnimationFileUrl = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/OnlyAnimation.txt”)!!
it(“throws a TextParseException”) {
failsWith(javaClass<TextParseException>()) {
convertToYaml(Paths.get(onlyAnimationFileUrl.toURI())!!)
}
}
}
on("loading file with trailing newlines") {
val trailingNewlines = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/TrailingNewlines.txt”)!!
it(“loads normally and ignores trailing newlines”) {
val expected = “Frames:n” +
" - Index: 0n" +
" Bounds: [157, 184, 123, 189]n" +
" - Index: 1n" +
" Bounds: [187, 0, 177, 183]n" +
" - Index: 2n" +
" Bounds: [402, 181, 105, 189]n" +
“Animations:n” +
" - Name: Walkingn" +
" Frames: [0, 1, 2]n"
val output = convertToYaml(Paths.get(trailingNewlines.toURI())!!)
assertEquals(expected, output)
}
}
on("Loading a file with unnatural ordering") {
val unnaturalOrdering = javaClass<ConverterSpec>().getClassLoader()!!.getResource(“converter/UnsortedFrames.txt”)!!
it(“loads and sorts frames into natural order”) {
val expected = “Frames:n” +
" - Index: 0n" +
" Bounds: [157, 184, 123, 189]n" +
" - Index: 1n" +
" Bounds: [187, 0, 177, 183]n" +
" - Index: 2n" +
" Bounds: [402, 181, 105, 189]n" +
" - Index: 10n" +
" Bounds: [187, 0, 177, 183]n"
val output = convertToYaml(Paths.get(unnaturalOrdering.toURI())!!)
assertEquals(expected, output)
}
}
}
}}