Item refactoring

[simonc]

This PR reduces Item's dependencies and public API.

An important change is the way an item impacts its container. Calling Item.new should not cause the item's container to be modified. The container is now touched only when calling container.item ... or container.items = .... It moves the responsibility from Item to ItemContainer where it belongs.

[simonc]

@andi Do you want to review the PR or may I merge it?

[andi]

do we break the API with it?

[simonc]

Yes, it removes the Item#autogenerate_item_ids? method and changes Item behavior. The real change is that the following code will not modify the container:

container = ItemContainer.new
item = Item.new(container, :key, 'url', 'name', container_id: 'hello')

while this two examples will:

container = ItemContainer.new
item = Item.new(container, :key, 'url', 'name', container_id: 'hello')
container.items = [item] # changes container id

container.item :key, 'url', 'name', container_id: 'hello' # changes container id

[andi]

fine with me, go ahead and merge :-)

On Tue, Apr 1, 2014 at 3:05 PM, Simon Courtois notifications@github.comwrote:

Yes, it removes the Item#autogenerate_item_ids? method and changes Item behavior. The real change is that the following code will not modify the container:

container = ItemContainer.newitem = Item.new(container, :key, 'url', 'name', container_id: 'hello')

while this two examples will:

container = ItemContainer.newitem = Item.new(container, :key, 'url', 'name', container_id: 'hello')container.items = [item] # changes container id container.item :key, 'url', 'name', container_id: 'hello' # changes container id

— Reply to this email directly or view it on GitHubhttps://github.com/codeplant/simple-navigation/pull/161#issuecomment-39202330 .

[simonc]

Great! I'll try to be clearer about side effects for next PRs ;)

[simonc]

Damn! I need to add integration tests. In my issue-testing-app something is broken. I'll dig into it and fix it.

[simonc]

Fixed.