Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

Ruby Ruby Booleans Build a Simple Todo List Program Part 4: Removing Todo Items

Posted by Unsubscribed User
Unsubscribed User
11,042 Points

Why we defined the remove_item method in that way?

Hi! I was looking at the remove_item method definition... there is a reason for writing this:

  def remove_item(name)
    index = 0
    found = false
    todo_items.each do |todo_item|
      if todo_item.name == name
        found = true
      end
      if found
        break
      else
        index += 1
      end
    end
    if found
      @todo_items.delete_at(index)
      return true
    else
      return false
    end
  end

instead of this? (my litte-bit-shorter version)

  def remove_item(name)
    index = 0
    found = false
    @todo_items.each do |item|
      if item.name == name
        found = true
        break
      else
        index += 1
      end
    end
    if found
      @todo_items.delete_at(index)
      return true
    else
      return false
    end
  end

I mean, why writing something like this?

if something
  found = true
end

if found
  other_statements
end

There is something that I am missing?

Another little thing: just for exercise I made this real-shorter version and it seems working: it is safe to delete an item inside an array we're looping on? &-)

  def remove_item(name)
    index = 0
    @todo_items.each do |item|
      if item.name == name
        @todo_items.delete_at(index)
        return true
      elsif (index + 1 == @todo_items.count) #if we reached the last item...
        return false
      else
        index += 1
      end
    end
  end

Thanks for your time! :-)

4 Answers

Seth Kroger
Seth Kroger
56,413 Points
Seth Kroger
Seth Kroger
56,413 Points

I don't think there's a reason other than personal coding style for the break statement to be one way or the other.

Removing a list's item outside of the each loop, however, has a good reason for it. Changing a list within each is a bit dangerous, particularly adding and removing items because it can cause each to repeat or skip items. (Remove the third item, and you'll skip the fourth because it's now the third.) It works in this instance because you're only removing one item and exiting the loop, but it's not something I'd get into the habit of.

Valentin Famelart
Valentin Famelart
19,190 Points
Valentin Famelart
Valentin Famelart
19,190 Points

As I found out throught the video, there is an even more simpler/smaller version with the delete_if method. As it is some native ruby code, I don't think it will skip items. I've tried to implement this and it apparently works quite fine!

def remove_item(name)
  deleted = false
  todo_items.delete_if {|item| deleted = true if item.name == name}
  return deleted
end
Dylan Parker
Dylan Parker
15,624 Points

you can also write it using do and end for the code block if you wanted to keep your code the same visually

def remove_item(name)
  deleted = false
  todo_items.delete_if do
     |item| deleted = true if item.name == name
  end
  return deleted
end
Unsubscribed User
Unsubscribed User
11,042 Points
Unsubscribed User
Unsubscribed User
11,042 Points

Are you meaning that an array element is directly removed from the ram when I call the delete_at method?

Unsubscribed User
Unsubscribed User
11,042 Points
Unsubscribed User
Unsubscribed User
11,042 Points

So if we remove the second to last array element, in the next loop the pointer goes out of the array what we get is a buffer over-read?