trjhtr

I'm learning Kotlin and I made a todo program (in command line) with Kotlin and Maven as my build tool.

Full project.

Description

The idea of my program is to read a file (path defined in a config file) that contains the list of tasks.

Since it's a project for practice purpose, the content of the file is quite simple and looks like this:

foo;true
bar;false
foobar;true

2 columns:

• Description of the task


• Status of the task

The only actions I've created are the following:

• 
  list the tasks (on the standard output)


• 
  add a new task (by writing a new line on the file)


• 
  finish a task (by setting the status of the task to true)

There are also other options:

• 
  help: display the list of options in the standard output


• 
  config: use another configuration file


• 
  verbose: display the debug logs


• 
  version: display the version of the program

The code

I'm using commons CLI library to help me parse command line options passed in the arguments.

I represented a task with the following:

class Task(var name: String, var isDone: Boolean = false) 
 // Override toString to display the task in "human"
 override fun toString(): String 
 var done = ""
 if (isDone) 
 done = "X"
 
 return "$name t$done"
 
 // function to transform the task in a line to be written in the file
 fun toLine(): String 
 return "$name$SEPARATOR$isDone"
 

In order to transform a line from my task file into a Task instance, it's quite straightforward:

fun parse(line: String): Task 
 val array = line.split(SEPARATOR)
 if (array.size < 2) 
 return Task("", false)
 
 return Task(sanitize(array[0]), sanitize(array[1]).toBoolean())


fun sanitize(s: String): String 
 return s.replace("n", "")

I created an interface Cmd which represents a command:

interface Cmd 
 fun getOption(): Option
 fun isEnabled(line: CommandLine): Boolean
 fun getOptionValue(line: CommandLine): String?

I also created a ActionCmd which represents an action command, like adding, finishing, or listing the tasks. This ActionCmd implements the Cmd interface:

interface ActionCmd: Cmd 
 fun execute(p: Path, arg: String)

In order to avoid duplicating the code for all my commands, I created an abstract class AbstractCmd that uses the commons-CLI classes:

abstract class AbstractCmd: Cmd 
 override fun isEnabled(line: CommandLine): Boolean 
 return line.hasOption(getOption().longOpt)
 

 override fun getOptionValue(line: CommandLine): String? 
 return line.getOptionValue(getOption().longOpt)
 

Finally, the action commands are the following:

class ListCmd : AbstractCmd(), ActionCmd 
 private val option = Option("l", "list", false, "print the list of tasks")
 override fun getOption(): Option 
 return option
 

 override fun execute(p: Path, arg: String) 
 val tasks = ArrayList<Task>()
 val stream = Files.newBufferedReader(p)
 stream.buffered().lines().forEach line -> tasks.add(parse(line)) 
 tasks.forEach println(it) 
 


class AddCmd: AbstractCmd(), ActionCmd 
 private val option = Option("a", "add", true, "add a task")
 override fun getOption(): Option 
 return option
 

 override fun execute(p: Path, arg: String) 
 // I have to add the empty string, otherwise, it's not going to add a new line
 p.toFile().appendText("" + Task(arg).toLine())
 


class FinishCmd : AbstractCmd(), ActionCmd 
 private val option = Option("f", "finish", true, "finish a task")
 override fun getOption(): Option 
 return option
 

 override fun execute(p: Path, arg: String) 
 val task = Task(arg)

 val tasks = ArrayList<Task>()
 val readerStream = Files.newBufferedReader(p)
 readerStream.buffered().lines().forEach line ->
 val t = parse(line)
 if (t.name == task.name) 
 t.isDone = true
 debug("Finishing task: $t")
 
 tasks.add(t)
 
 File(p.toString()).printWriter().use out ->
 tasks.forEach out.println(it.toLine()) 
 
 

Does my code follow common best practices or am I going the wrong way?

Here are some recommendations:

Use if as expression when it's appropriate.

var done = ""
if (isDone) 
 done = "X"

The better way is to write:

val done = if (isDone) "X" else ""

Also try to use val as much as it's possible. That will make your code simpler and more reliable.

Don't write obvious comments like comments for toString or toLine functions.

I would rename functions toLine and parse to serialize/deserialize. Or I'd use just toString + parse.

Using global variables like SEPARATOR is a bad idea.

I'd recommend use data class for Task and Commands.

private val option = Option("a", "add", true, "add a task")
override fun getOption(): Option 
 return option

It equals to

val option = Option("a", "add", true, "add a task")

Also it's a good practice to avoid using in functions many arguments with the same type. If you can't avoid you still can name them explicitly by writing method(param = arg1..)

This looks weird

p.toFile().appendText("" + Task(arg).toLine())

Try to use more filters, maps, and other hi-order functions over collections

val tasks = ArrayList<Task>()
val stream = Files.newBufferedReader(p)
stream.buffered().lines().forEach line -> tasks.add(parse(line)) 
tasks.forEach println(it) 

This is just:

p.toFile().readLines().map parse(it) .forEach 
 println(it)

Another example:

 val tasks = ArrayList<Task>()
 val readerStream = Files.newBufferedReader(p)
 readerStream.buffered().lines().forEach line ->
 val t = parse(line)
 if (t.name == task.name) 
 t.isDone = true
 debug("Finishing task: $t")
 
 tasks.add(t)
 

Is just

val tasks = p!!.toFile().readLines().map parse(it) 
commands.filter it.name == task.name .forEach it.isDone = true 

If you want I can rewrite your program on my taste (as example).