Anagram Generator

Question

This is a program that takes lines from 'wordList' and prints it's anagrams, found in the dictionary.

Please give me feedback regarding good coding practices.

import java.io.File

fun main(arg: Array<String>) {
    val wordList =
            File("wordList.txt")
                    .readLines()
    for (word in wordList) {
        val sortedWordChar =
                word
                        .toCharArray()
                        .sorted()
        val dictionary = File("dict.txt")
                .readLines()
        dictionary
                .filter { it.length == word.length }
                .map { Pair(it, it.toCharArray().sorted()) }
                .filter { it.second == sortedWordChar }
                .forEach { print("${it.first} ") }
        println()
    }
}

Answer 1

    for (word in wordList) {
        ...
        val dictionary = File("dict.txt")
                .readLines()
        ...
    }

On the assumption that dict.txt isn't changing, I don't see a reason to keep this in the loop. Disk caching means that it might not have too bad a performance hit nowadays, but it's still not a good idea to do an expensive operation multiple times if you can store the result.

---

                .map { Pair(it, it.toCharArray().sorted()) }
                .filter { it.second == sortedWordChar }
                .forEach { print("${it.first} ") }

Since you don't need it.second past the filter, it would be simpler to write

                 .filter { it.toCharArray().sorted() == sortedWordChar }
                 .forEach { print("${it} ") }

---

On style, I'm not sure what's going on with the indentation. In particular,

        val sortedWordChar =
                word
                        .toCharArray()
                        .sorted()

looks weird to me. If it's necessary to split into multiple lines (which I'm not convinced of, given that essentially the same expression is on a single line in the map), wouldn't one tab be sufficient indentation rather than two?

Answer 2

let's continue :-)

sequence

filter, map and flatmap are implemented in roughly the same way:

1. create a new List
2. perform operations
3. Store results in the new list
4. return the new list.

this means that for every filter, map and flatmap a new list is created and all the elements are iterated. To increase performance, you can change it into a Sequence.

val message = dictionary.asSequence()
            .filter { it.length == word.size }
            .filter { it.toSortedCharArray() contentEquals word }
            .joinToString(separator = " ")

repeated code

If you look at the code, you see that you get sortedCharArrays from both files. You can therefor create a function:

fun File.readSortedCharArrays() = readLines().map(String::toSortedCharArray)

so you get

val dictionary = File("dict.txt").getSortedCharArrays()

File("wordList.txt").getSortedCharArrays().forEach { word ->
    val message = dictionary.asSequence()
            .filter(CharArray::size::equals)
            .filter { it contentEquals word }
            .joinToString(separator = " ")
    println(message)
}

joinTo

finally, joinToString is a wrapper around a function that takes more arguments: joinTo. Here you can specify where you like to add the created string to. In our case it is System.out.

altogether

fun String.toSortedCharArray() = toCharArray().apply(CharArray::sort)
fun File.getSortedCharArrays() = readLines().map(String::toSortedCharArray)

val dictionary = File("dict.txt").getSortedCharArrays()

File("wordList.txt").getSortedCharArrays().forEach { word ->
    dictionary.asSequence()
            .filter(CharArray::size::equals)
            .filter { it contentEquals word }
            .joinTo(System.out, " ")
}

I know, it looks less beautiful, but it is better code... Ps, if you want to have it as a list of Strings, instead of printing it out, you need to use flatmap:

fun String.toSortedCharArray() = toCharArray().apply(CharArray::sort)
fun File.getSortedCharArrays() = readLines().map(String::toSortedCharArray)

val dictionary = File("dict.txt").getSortedCharArrays()
val result : List<String> = File("wordList.txt").getSortedCharArrays().flatMap{ word ->
    dictionary.asSequence()
            .filter { it.size == word.size }
            .filter { it contentEquals word }
            .toList()
}.map(Any::toString)

Answer 3

1. Using sorted() returns a new object but such isn't necessary in this case. You can use sort() directly on the CharArray.

2. You can reduce duplicated code by using classes and/or extension functions. e.g.:

   fun String.toSortedCharArray() = toCharArray().apply(CharArray::sort)
   

3. I recommend building each line and printing it atomically instead of printing it in segments. e.g.:

   val message = dictionary.filter { it.length == word.length }
           .filter { it.toSortedCharArray() contentEquals sortedCharArray }
           .joinToString(separator = " ")
   println(message)
   

   This also prevents printing a trailing space.

4. You don't need to re-read "dict.txt" as it doesn't change with each word.

Altogether:

val dictionary = File("dict.txt").readLines()
val wordList = File("wordList.txt").readLines()
fun String.toSortedCharArray() = toCharArray().apply(CharArray::sort)
for (word in wordList) {
    val sortedCharArray = word.toSortedCharArray()
    val message = dictionary.filter { it.length == word.length }
            .filter { it.toSortedCharArray() contentEquals sortedCharArray }
            .joinToString(separator = " ")
    println(message)
}

Answer 4

I think this is a very small thing and may be just personal preference, but what about keeping it consistent and using a forEach loop everytime.

File("wordList.txt").readLines().forEach {
    val sortedWordChar = it
    ...
}

I also changed some indentation there, it feels a lot cleaner to me this way.

With what mfulton26 and Peter Taylor said, my altogether would look like this

val dictionary = File("dict.txt").readLines()
fun String.toSortedCharArray() = toCharArray().apply(CharArray::sort)
File("wordList.txt").readLines().map { it.toSortedCharArray() }.forEach {
    val sortedCharArray = it
    println(dictionary.filter { it.length == word.length }
            .filter { it.toSortedCharArray() contentEquals sortedCharArray }
            .joinToString(separator = " "))
}

Notice how I also deleted the val of message and just put it into the println function directly and mapped the lines to sorted CharArray's before looping.