Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reset! command #3

Merged
merged 1 commit into from
Oct 4, 2016
Merged

Add reset! command #3

merged 1 commit into from
Oct 4, 2016

Conversation

atomaka
Copy link
Contributor

@atomaka atomaka commented Oct 4, 2016

Allow users to clear our current object cache. This is particularly useful when Unique is being used in combination with RSpec and Faker.

@b264
Copy link
Owner

b264 commented Oct 4, 2016

I was thinking about this sort of functionality as well, but this seems like the poster-child use-case for a stateful class. I was thinking something like

unique_set1= Unique.new{ 1 }
unique_set2= Unique.new{ 1 }

unique_set1.take
 => 1
unique_set1.take
Unique::NoUniqueObjects: An unused, unique object could not be found in 4096 tries
unique_set2.take
 => 1
unique_set2.take
Unique::NoUniqueObjects: An unused, unique object could not be found in 4096 tries

Unique.config.max_tries= 10

...and possibly a module-level scope as well (or not .. ?? )
Unique.next!{ 1 }
 => 1
Unique.next!{ 1 }
Unique::NoUniqueObjects: An unused, unique object could not be found in 4096 tries

I think this could do everything as a reset command, plus more, in a more intuitive manner. Any thoughts about this? The downsides of the module-level approach is that it could use a lot of memory if left running for a long time, whereas the class approach would work very well for testing and shouldn't leak memory. If it's used outside of testing, anyway. The purpose of this gem when written was mainly for testing and db seeding with faker.

@b264
Copy link
Owner

b264 commented Oct 4, 2016

I tested this approach with RSpec and it seems that using the module-level approach with @@instances is the only way it will work. The class approach could be added later if needed. RSpec seems to give Unique amnesia when used the other way.

@@ -0,0 +1,29 @@
describe Unique do
context '.reset!' do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be added to spec/unique/unique_spec.rb instead of here so CI will run this. It looks good otherwise. https://travis-ci.org/b264/unique/jobs/164806377#L165

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rake or rake default should run the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and rebased.

@atomaka
Copy link
Contributor Author

atomaka commented Oct 4, 2016

Stateful makes sense, but I also had concerns about that functioning with RSpec.

@b264
Copy link
Owner

b264 commented Oct 4, 2016

:shipit:

@b264 b264 merged commit a115af8 into b264:master Oct 4, 2016
@b264
Copy link
Owner

b264 commented Oct 4, 2016

released as 0.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants