search

bkconrad / wasabi

MIT License
31 stars 1 forks source link

Group#addObject, Group#removeObject, Wasabi#addObject, Wasabi#removeObject interactions #29

Closed bkconrad closed 10 years ago

bkconrad commented 10 years ago

27 and #28 expose a couple of problems.

I think Group#addObject should implicitly Wasabi#addObject, and that Wasabi#removeObject should Group#removeObject.

Does that sound ok?

mreinstein commented 10 years ago

Sounds good! I think this is a documentation issue too. Curious about the behavior of trying to double add and double remove.

mreinstein commented 10 years ago

another thing to consider; technically this is another breaking api change depending on how you implement it. :/ ideally you can do this by adding to the API and then bump the minor rev number.

bkconrad commented 10 years ago

I think you're right when you suggest using a hash instead of an Array. I only used an Array for the .length property, but that is definitely outweighed by the complexity gains and set behavior.

bkconrad commented 10 years ago

another thing to consider; technically this is another breaking api change

Ah, because it's changing the semantics of the methods. But, it is technically compatible, and also a bugfix. Calling Wasabi#removeObject() after Group#removeObject() should work fine after the change. Is that still OK for a minor bump?

(I'm ok with being technical about things, I want to do it as correctly as possible).

mreinstein commented 10 years ago

But, it is technically compatible, and also a bugfix

Think of it this way. Let's say the 1.0.0 release has been out for a few weeks and several people are using it. If you were to introduce this change as a 1.1.0, people that have 1.x in their package files would get an update. Would any of the changes in 1.1 break behavior that someone else is depending on? If so, it'd be 2.0.0. If it doesn't break behavior (e.g., my code has Wasabi.removeObject(player); group.removeObject(player); and calling these 2 has the same effect, then 1.1.0 is valid.

bkconrad commented 10 years ago

Ok, I'm going to reimplement the group list as a hash, and the object registry is already a hash, so after I make those changes then any existing working code would still work.

(I rolled back the version number to 0.2.0, btw)

mreinstein commented 10 years ago

Sounds like a good plan. :+1:

Usually I like to maintain the api for a while, and when enough things have changed, I'll bump the major api number, and put all the breaking changes into 1 release. It's considered responsible to not break the API too frequently, unless there's a really good reason. (clearing out a lot of built up deprecation over time and fundamental changes to the underlying system are both pretty good reasons for example.)

A lot of npm projects also use the 0.x series to denote it's still very alpha/betaish but this isn't universally true by any stretch.