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

suggest a moops version of Yanick's deduper as an example #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mokko
Copy link
Contributor

@mokko mokko commented Jan 26, 2014

I didn't talk to Yanick. Just thought the recent award-winning piece of code might might be a good demo for moops's clean code. Perhaps also for performance.

I guess the file should make better reference to original author in case you want to include it. I just wanted to run it by you to see what you think.

@tobyink
Copy link
Owner

tobyink commented Jan 30, 2014

traits    => ['Counter'],       #doesn't work in Moops?

This isn't going to work in a Moo class. Moops uses MooX::HandlesVia to implement attribute traits in Moo classes (because Moo doesn't have any native support for attribute traits). MooX::HandlesVia's documentation notes that because of the way Moo is implemented, this is only really useful for attributes where the values are references (i.e. arrayrefs, hashrefs, etc).

Also, I think the hash_size stuff is broken. (The trigger for the hash_size attribute in Deduper tries to call Deduper::File->hash_size(...) but this will fail, because it's an object method, not a class method.)

Overall, I think it might be too long to serve as a useful example.

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