Code review: TyphoonGenericStack

[jasperblues]

@cesteban

• (id)pop
  {
  id element = [_storage lastObject];

  [_storage removeLastObject]; <----- This can cause array index out of bounds, wrap in if.
  return element;
  }

[cesteban]

You are right. This out of bounds would never happen in the context of Typhoon stash, but this is a generic stack. I'll address this along with the concurrent issue.

[jasperblues]

I was just being (unnecessarily) pedantic ;)

Still - we might as well make it generic, since its only a one line change.

[cesteban]

Don't worry, that wasn't pedantic. The class is named GenericStack, so popping when empty must be supported and properly handled. I'm changing this.

[cesteban]

I made "failing" test for this, and it happens that when calling removeLastObject for empty array, no exception is thrown! From Apple documentation: removeLastObject raises an NSRangeException if there are no objects in the array..

I found this stack overflow discussion: http://stackoverflow.com/questions/14760542/nsmutablearray-removelastobject-exception

I protected the stack anyway, just in case. Should I directly push the changes, or do you prefer me to branch and pull request? It is one line of code and one added test.

[jasperblues]

I trust the quality of your work 100% - you've been allocated push access.

[jasperblues]

Also, feel free to push anytime. The general rules are:

Any experimental/controversial/long-running feature should be done on a branch, because:

• This kind of work might not be finished before someone else commits a useful feature. So would hold up a release.
• Experimental work might need to be rolled back, if we discover something that "ain't gonna fly".

Having said that, we practice 'continuous integration', so the sooner it goes to master the better. If it exposes a problem, we'll know about this early and deal with it. . . use your judgement to balance these concerns.

Any other work can go straight to master.

Don't be shy ;)

[cesteban]

Ok, thanks. I generally don't like pushing to master, but this is a pretty obvious one liner, so we should be safe.

[jasperblues]

Ok - whatever you prefer. I you want, you push you have access. Pull requests are also fine.

In any case - I really appreciate your contributions and feedback. They've been useful for my work, as well as for other Typhoon users, I'm sure.

[cesteban]

Thanks a lot, but I just did a few fixes, nothing really important ;)

[jasperblues]

What did I write above!?!? I meant to write:

"If you want, you have push access. Pull requests are also fine".