-
-
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 |
Hey Samuel - I taught Ruby to a few junior high kids this last summer. It went MUCH slower than I anticipated! Who's your target? Kid or Adults, and with prior programming experience or not? I think this example might work well for people who are already programmers, but otherwise it might be a little advanced.
I found that any talk of OOP was way overkill until they got some basics down with Ruby syntax. You'd be surprised, the concept of "variables" alone will blow a lot of non-programmers' minds. I think the authors of the pickaxe book mentioned this in their latest version too, so now they have a basics chapter before ever touching the higher concepts.
I think you have 4-6 hours of classroom material packed in here :)
I agree with @bellmyer. Even with several years of Basic, Pascal, and C programming under my belt (a long time ago) it took me quite a while to grok objects. Also, some suggestions regarding the existing content:
- IMO the 'due_on' param to the constructor should be a date; the responsibility of parsing it from a string is a specialized one and should not be in the constructor.
- class name should be "ToDo", not "Todo".
- default argument 'complete' 's default value should be false, not nil. Then there is no need for extra processing. If you want to ensure it's a boolean, you can say @complete = !!complete.
- I'd strongly discourage newbies (and anyone) from using any variables with "$" in them, even if they're thread-specific and not global. There's a method name equivalent that can be used instead.
- the implementation of self.complete? should probably be self.complete and not self.complete == true. It's rarely a good idea to test for a specific boolean value rather than the "truthy" equivalent (falsy = false or nil, true = anything else).
- the index param to complete should already be an integer; there should be no need to call to_i and doing so encourages the caller to do what shouldn't be done (not pass an integer).
- the API should use zero offset; if you want to translate to 1 offset in a UI that's fine but IMO not in code this low level (the 'index - 1' occurrences).
- the ToDo filespec should be initialized once as a constant, to model to the students the DRY principle and to simplify the code. Then you can simplify the other code, e.g.
File.readlines(TODO_FILESPEC).each { | line | self.todos << Todo.from_string(line.chomp) }
This would also eliminate the need for the 'a+' mode string, which probably should have been 'r'.
- Unless the students are already familiar with printf style formatting, I'd suggest using ordinary Ruby string interpolation instead; it's a little more intuitive and they're likely to see much more of it in Ruby code.
Hi Samuel,
I also agree with @bellmyer. There's a lot here--good stuff, but I'd want at least 1 hour plus time for questions to teach an OO-savvy programmer crowd (coming from Java, C#, C++, Python, etc.) If this is for a crowd that hasn't done OO it would take more, and for a non-programmer crowd this should be broken up into several lessons.
Are you planning on evolving the code by DRYing it as part of the talk? That might be a good idea. Or maybe a test or two...but then again, only 30 minutes is a very short time.
@bellmyer, @coderkevin: it's hard to explain and I'm still working on things. It might be that I spread out creating the code above across different things. There is a lot to what has been written, and yes everything in total is going to take between three and five hours, but could writing the above code from start to finish while explaining what I was doing take less then thirty minutes.
@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.
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
The point of this script isn't to show a perfectly designed script, but really more to highlight some of Ruby's basic concepts: OOP, iterators, conditional logic, string interpolation, regexes, file access, etc.
I have a time constraint of about 30 minutes for creating this script. Let me know if you think creating the above is reasonable in that time, and if there are things which might be simplified.
Thanks.