More rubyish Result class #34

Closed
wants to merge 2 commits into
from

Projects

None yet

2 participants

@jnunemaker

This adds #each, #size, #length, #count, and makes result Enumerable.

One note: I am running specs against cass 1.1. In order to get the specs to pass, I had to make sure that the keyspace check in setup_cassandra_connection in spec_helper was case insensitive. I did not include that in this pull request, but I certainly can if you are ok with it.

jnunemaker added some commits Nov 3, 2012
@jnunemaker jnunemaker Added Result#each and mixed in Enumerable.
Makes result more friendly. The assumption is you usually want an array of hashes. If you don't, you can use fetch_array or whatever.
727d100
@jnunemaker jnunemaker Make sure #size, #length, and #count work.
Ruby arrays and such typically respond to these. #rows doesn't feel obvious to me. I would think that would actually be an array of the rows, but I'm leaving it unchanged for backwards compatibility.
4815518
@kreynolds
Owner

A few comments:

  1. I don't agree that hashes should be the default and unchangeable assumption but I can't think of a clean way to make that a twiddle. Can you?
  2. #rows exists to map the property of the same name from the thrift CQL result. I'd be fine with a PR deprecating it's use and flipping to size/count/length, but it should not return an array or rows because it wouldn't know what format of row to return. For those cases I'd advocate the addition of to_a on the Result object. Thoughts?
@jnunemaker

Thanks so much for the response. I maintain several gems and I know it takes time, so I appreciate your time.

  1. What do you mean by default and unchangeable? Were you saying that in regards to picking hashes for #each? To me, it just adds simple flavor for what I have found to be my most common use case. We can leave the other fetch methods as is for those that need them, or even make them line up with each, say each_array, etc.

Do you find yourself using one of the other fetch methods more often? Or all of them relatively equally?

I suppose we could just rename #each to #each_hash or something like that, but again, I would advocate picking the most common usage for #each and then just leaving the other fetch methods as is, including fetch_hash.

Maybe I'm wrong, I just went with my gut and what I have found myself wanting.

  1. Gotcha. No prob. I am fine with leaving it as is. I just keep wanting to do size, count, or length instead of rows. I'm cool with the aliasing as is, if you are. I don't feel any urgent desire to deprecate or anything.
@kreynolds
Owner

My use cases are way more towards the time-series end of things where arrays are more useful. I wish there were a useful way to ask users which is more common and do whatever that is but maybe since that's not practical, you should just refactor this (now that I've pulled in the other request) and have it your way. You wrote it after all :)

@jnunemaker jnunemaker referenced this pull request Nov 14, 2012
Merged

Enumerable #38

@jnunemaker

Closing in favor of #38.

@jnunemaker jnunemaker closed this Nov 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment