Proposal: Enumerable#squeeze, String#rotate etc #80
cielavenir posted onGitHub
https://github.com/cielavenir/is07kuno/blob/master/libciel.rb
I'm wondering if I'm going to publish a gem, but I'm glad if merged into facets.
For now, I published my own gem: https://github.com/cielavenir/ruby_libciel/ http://rubygems.org/gems/libciel
I will look them over and get back to you. Thanks.
Hi @cielavenir,
Thanks for your submissions! I looked them over and have some comments. In order to make it easier to incorporate your contributions into Facets, it would help if you could make the coding style and structure consistent with the rest of Facets. You should fork Facets and add your methods on a branch. Some guidelines for your methods (see CONTRIBUTE.md for more):
- be broken into separate files consistent with the structure of the rest of Facets
- be documented in the style of the rest of Facets; https://github.com/rubyworks/facets/blob/master/lib/core/facets/array/uniq_by.rb provides a good example. You should explain each method and provide one or more examples.
- have associated Lemon unit tests (placed in the test folder)
Here are my comments on your methods functionality:
Object#extract
looks good!Kernel#zip
I'm not sure what this does-- I also don't think it should be onKernel
, since this is mixed in everywhere. Can you provide an example use-case?Enumerable#squeeze
looks good!Enumerator::Lazy#squeze
looks good!String#rotate/rotate!
looks good!Hash#exists_rec?
I like this concept, but I think it needs to be renamed (fetch_nested
?). It returns the actual value rather than a Boolean, so a?
is not a good last entry. I also think the arglist should be splatted so that it's not necessary to pass an array. Finally, I think it should accept a block likeHash#fetch
that provides an alternative in the case that the record doesn't exist.
def fetch_nested *keys
begin
keys.reduce(self) { |accum, k| accum.is_a?(Hash) && accum.fetch(k) }
rescue KeyError, NoMethodError
block_given ? yield(*keys) : nil
end
end
Hash#patch
I can't tell, does this method do anythingHash#merge
doesn't do?Array#permutation2
can you be more specific about what this does? I tried calling bothpermutation(4)
andpermutation2(4)
on[1,2,3,4,5]
and the results were identical.
In general you've got good stuff here and I'd like to see it included!
Thank you for comments. You can see testcases here: https://github.com/cielavenir/ruby_libciel/blob/master/spec/libciel_spec.rb (unfortunately this is written using rspec, so it seems I need to convert it to lemons)
- DBI Actually this is something I used in my database experiment. Anyway it might not have good quality to be included to facets... At least I can confirm dbd-sqlite3 stopped working in Ruby 2.0.
- Kernel#zip
Perhaps I should add a testcase:
zip([[1,2,3],[4,5]]).should eq [[1, 4],[2, 5],[3, nil]]
. [[1,2,3],[4,5]].transpose fails. Well, here,[1,2,3].zip([4,5])
can the same thing. So it doesn't need to be included... - Array#permutation2
[1,1,2,3].permutation2
is different from[1,1,2,3].permutation
. - Hash#patch You are correct. Removed in 0.0.0.3.
- Enumerator::Lazy#squeze
Thank you. By the way if we unsupport enumerable/lazy and force backports gem,
Lazy.class_eval
can be changed toclass Lazy
, which is cleaner. How would you like? - Hash#fetch_nested Your implementation is brilliant...
libciel was updated to 0.0.0.3. Now source structure should be OK. Next task should be facets-style documentation...
The best way to go about this, if you have the time and you'd really like to see these methods make into Facets, is to fork the Facets repo, then add one method at a time, with documentation, Lemon unit test and QED demo, and then commit and send a pull request. That way we can evaluate/discuss/merge each method individually on its own merits.
We have Array.zip
class method, instead of Kernel#zip
. I can see how Kernel#zip
can be convenient, but the same can be said about many Enumerable and Array methods. In such cases, it's best for the developer to define a private method where they want the convenience, e.g.
def zip(*a); Array.zip(*a); end
For the DBI extensions, I would suggest submitting them to the DBI project (I assume that's the official project). Facets isn't the right place for those --it is all about Ruby's core and standard libraries.
Note, I just read the the first issue in the DBI project and it appears the project is no longer being maintained, so there is an opportunity there, if you are a user, to bring your own influence to the project.
I handed in my master thesis...
Very sorry but I'm not sure if I'm doing the right thing.
#fork facets repo
git clone git@github.com:cielavenir/facets.git
cd facets
sudo bundle
lemons test
produces:
/Library/Ruby/Gems/2.0.0/gems/lemon-0.9.1/lib/lemon/cli.rb:28:in `cli': undefined method `cli' for Test::Runner:Class (NoMethodError)
from /Library/Ruby/Gems/2.0.0/gems/lemon-0.9.1/bin/lemons:8:in `<top (required)>'
from /usr/bin/lemons:23:in `load'
from /usr/bin/lemons:23:in `<main>'
This will be fixed via https://github.com/rubyworks/facets/pull/86. Now I'm going to do branching.
I haven't tried the lemons
command in some time. I'll have to look at that. But it's a thin wrapper around rubytest
, so you can use that instead. However, it's much easier just to run rake test
.
@boostio funded this issue with $10. Visit this issue on Issuehunt
It seems the proposed methods already exist in the code base?