Refactor stringify for HStore for consice method

[leaexplores]

This PR treats the small refactor of HStore inside meta-issue of pr #20 .

Do note that the hard class check using value.class == type is required.

Else you'll run into issues like Can't typecast: Bignum with unit tests.

Thanks!

[jackc]

I get why the value.class == type is necessary. What I don't see is where that would ever be true where value.is_a?(type) could be false. That is, if value.class == type is true value.is_a?(type) will also always be true.

[leaexplores]

Yeah mhm, I think I finally found the issue.

The bug wasn't inside the is_a? and class compare.

It was because I was re-using the value class type instead of the _tostring type.

If we receive IE a Bignum object.

We should use the matching type to serialize the object instead of the original one. The order was reversed in the buggy code :(. As per it only worked when we had the class == is_a?.class.

Should work ok now and we only have is_a? :)

Thanks for sticking into it.

[jackc]

I looked a bit closer and found two more issues. The previous version encoded dates with value.to_s(:db). The new version would only do value.to_s. The tests still pass, so it can still be round-tripped, but it may have backwards compatibility issues.

Another is there is no reason to use select to find a single value (line 88). Use detect or find instead when searching for a single value.

A last thought, is when I was going back and forth between the old version and the new version, I really had to think a bit to understand the new version. It definitely is shorter, but I wonder if staying with a boring and longer if statement really might be clearer.

[leaexplores]

We're hitting a lot of nits. It's a great opportunity to add some more tests :).

I'll open a PR quick to test that :db option is called on to_s calls.

[leaexplores]

Closing this one as it's not really a good improvement. We are compicating things for the sake of saving a few lines of code.