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 trialUnsubscribed User
11,042 PointsWhy 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
56,413 PointsI 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
19,190 PointsAs 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
15,624 Pointsyou 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
11,042 PointsAre you meaning that an array element is directly removed from the ram when I call the delete_at method?
Unsubscribed User
11,042 PointsSo 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?