-
-
Save samullen/213df109bbffb2240d2e to your computer and use it in GitHub Desktop.
#!/usr/bin/env ruby | |
require "date" | |
class Todo | |
attr_accessor :complete | |
attr_reader :name, :due_on | |
def initialize(name, due_on=nil, complete=false) | |
@name = name | |
@due_on = due_on | |
@complete = complete | |
end | |
def self.from_string(string) | |
todo = string.split("::") | |
complete = todo[0] == "x" | |
name = todo[1] | |
due_on = todo[2] ? Date.parse(todo[2]) : nil | |
self.new(name, due_on, complete) | |
end | |
def complete! | |
self.complete = true | |
end | |
def complete? | |
self.complete == true | |
end | |
def to_s | |
complete = self.complete? ? "x" : "-" | |
[complete, self.name, self.due_on].join("::") | |
end | |
end | |
class TodoList | |
attr_reader :todos | |
TODOPATH = File.join(ENV["HOME"], "todos.txt") | |
def initialize | |
@todos = [] | |
load_todos | |
end | |
def all | |
self.todos | |
end | |
def add(name, due_on) | |
self.todos << Todo.new(name, due_on) | |
save_todos | |
end | |
def complete(index) | |
self.todos[index].complete! | |
save_todos | |
end | |
def move(start_index, end_index) | |
todo = self.todos.delete_at(start_index) | |
self.todos.insert(end_index, todo) | |
save_todos | |
end | |
def delete(index) | |
self.todos.delete_at(index) | |
save_todos | |
end | |
private | |
def load_todos | |
File.open(TODOPATH, "a+") do |file| | |
file.each do |line| | |
self.todos << Todo.from_string(line.chomp) | |
end | |
end | |
end | |
def save_todos | |
File.open(TODOPATH, "w") do |file| | |
self.todos.each do |todo| | |
file.puts todo.to_s | |
end | |
end | |
end | |
end | |
todo_list = TodoList.new | |
def list_todos(todo_list) | |
todo_list.all.each_with_index do |todo, index| | |
complete = todo.complete? ? "x" : " " | |
line = "#{index + 1}. [#{complete}] #{todo.name}" | |
line += " - #{todo.due_on}" if todo.due_on | |
puts line | |
end | |
end | |
def puts_title(title) | |
puts title | |
puts "-" * title.length | |
end | |
puts | |
case ARGV[0] | |
when "list" | |
puts_title "Listing todos" | |
list_todos(todo_list) | |
when "add" | |
puts_title "Adding a todo" | |
todo_list.add(ARGV[1], ARGV[2]) | |
list_todos(todo_list) | |
when "complete" | |
puts_title "Completing todo #{ARGV[1]}" | |
todo_list.complete(ARGV[1].to_i - 1) | |
list_todos(todo_list) | |
when "move" | |
puts_title "Moving todo #{ARGV[1]} to #{ARGV[2]}" | |
todo_list.move(ARGV[1].to_i - 1, ARGV[2].to_i - 1) | |
list_todos(todo_list) | |
when "delete" | |
puts_title "Deleting todo #{ARGV[1]}" | |
todo_list.delete(ARGV[1].to_i - 1) | |
list_todos(todo_list) | |
else | |
puts <<-EOL | |
Usage: todo.rb <option> [option argument(s)] | |
list # List the todo items in your list | |
add <name> [due date] # Add new todo to your list with optional due date | |
complete <position> # Mark a todo complete | |
move <pos> <new pos> # Move a todo to a different position | |
delete <position> # Remove a todo at a position | |
help | |
EOL | |
end | |
puts |
Thanks, @keithrbennet. Lots of good stuff there, and I really appreciate you taking the time necessary to get such great feedback.
@samullen The only advice I'll offer is to drop the the regex. Just parse the string with a split on some whitespace character or something simple. Move list_todos
into the object itself, perhaps ahh.....
class Todo
def self.from_string(string)
complete, name, due_on = string.split("\t")
complete = complete == "x"
self.new(name, due_on, complete)
end
def to_s
[(complete ? "x" : "-"), name, due_on].join("\t")
end
end
Regex sucks, its hard, and its very rare. Don't scare Ruby newbiews with it. Yeah, Ruby has regex, but every other language has it, too.
Oh... and I disagree with ToDo
, it's just awkward. There are plenty of usages of "todo" without a hyphen or a space, so just keep it one word.
Quick annotation, only paying attention to details of the code rather than the whole of it. I can go back over it later with that in mind:
#!/usr/bin/env ruby
# Is that even necessary to require?
require "date"
class Todo
attr_accessor :complete
attr_reader :name, :due_on
# Avoid nil like the plague. Even if you did use it, these will
# still be nil if you leave off the default there
def initialize(name, due_on=nil, complete=nil)
@name = name
# Short circuiting can come in handy here:
# @due_on = due_on && Date.parse(due_on)
@due_on = due_on ? Date.parse(due_on.to_s) : nil
# Just do a boolean check on it, no reason to || false.
@complete = complete || false
end
def self.from_string(string)
# I'd agree with avoiding regex unless absolutely necessary.
# It tends to get hairy fast.
string.match(/\A(x|-) (.+?)\s*(\d{4}-\d\d-\d\d)?\z/)
# Look into the english regex terms, assume perlisms like $1 are off-limits
complete = $1 == "x"
name = $2
due_on = $3
self.new(name, due_on, complete)
end
def complete!
self.complete = true
end
# You can just use self.complete here
def complete?
self.complete == true
end
end
class TodoList
attr_reader :todos
def initialize
@todos = []
load_todos
end
# Look into Enumerable, it'll allow you to iterate over things by
# defining an each method, and a comparator (<=>)
def all
self.todos
end
# Look into after-method hooks so you can abstract the save_todos
# method into being executed after certain methods.
def add(name, due_on)
self.todos << Todo.new(name, due_on)
save_todos
end
def complete(index)
self.todos[index.to_i - 1].complete!
save_todos
end
# Whole words won't kill anyone. Type them out instead of using odd
# abbreviations.
def move(start_idx, end_idx)
todo = self.todos.delete_at(start_idx.to_i - 1)
self.todos.insert(end_idx.to_i - 1, todo)
save_todos
end
def delete(index)
self.todos.delete_at(index.to_i - 1)
save_todos
end
private
def load_todos
# Make the file path a constant in the head of this
File.open(File.join(ENV["HOME"], "todos.txt"), "a+") do |file|
# Consider the use of map instead:
#
# self.todos.concat file.map { |line| Todo.from_string(line.chomp) }
file.each do |line|
self.todos << Todo.from_string(line.chomp)
end
end
end
def save_todos
File.open(File.join(ENV["HOME"], "todos.txt"), "w") do |file|
self.todos.each do |todo|
file.puts "%c %s %s" % [todo.complete? ? "x" : "-", todo.name, todo.due_on]
end
end
end
end
todo_list = TodoList.new
def list_todos(todo_list)
# If you do that Enumerable bit above, you can get rid of all
todo_list.all.each_with_index do |todo, index|
# This ternary is repeated multiple times. Abstract it to a method instead
line = "%.2d [%c] %s" % [index + 1, todo.complete? ? "x" : " ", todo.name]
# use <<, as it's faster
line += " - %s" % todo.due_on.strftime("%b %d, %Y") if todo.due_on
puts line
end
end
puts
# Might want to chomp that, newlines make for a bad time.
case ARGV[0]
when "list"
# nifty method idea for formatting:
#
# def puts_title(string)
# puts string
# puts '-' * string.length
# end
puts "Listing todos"
puts "-------------"
list_todos(todo_list)
when "add"
puts "Adding a todo"
puts "-------------"
todo_list.add(ARGV[1], ARGV[2])
list_todos(todo_list)
when "complete"
puts "Completing todo #{ARGV[1]}"
puts "------------------"
todo_list.complete(ARGV[1])
list_todos(todo_list)
when "move"
puts "Moving todo #{ARGV[1]} to #{ARGV[2]}"
puts "--------------------"
todo_list.move(ARGV[1], ARGV[2])
list_todos(todo_list)
when "delete"
puts "Deleting todo #{ARGV[1]}"
puts "----------------"
todo_list.delete(ARGV[1])
list_todos(todo_list)
else
puts <<-EOL
Usage: todo.rb <option> [option argument(s)]
list # List the todo items in your list
add <name> [due date] # Add new todo to your list with optional due date
complete <position> # Mark a todo complete
move <pos> <new pos> # Move a todo to a different position
delete <position> # Remove a todo at a position
help
EOL
end
puts
Great! Thanks everyone for your input. I implemented most of the suggestions, but not all; some was left in for the purpose of illustration, even if not the "Ruby Way"(tm).
I really appreciate the help and constructive feedback.
Looks great. I am a bit surprised that people feel regex is seldom used. It is a good skill to have, but definitely a subject in and of itself :P
@bellmyer and as a followup, the assumption is that those participating will have some knowledge of programming. Maybe not OOP, but at least an understanding of variables, control structures, etc.