https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34828 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #2 from David Cook <dcook@prosentient.com.au> --- I'm intrigued by this one. It looks practical and high performance. However, it also feels a bit unnatural. It's more natural to have something like $metadata->get_normalized_upc. -- Looking at your patches, I notice that Koha::MetadataExtractor::MARC::MARC21 doesn't have any internal state, so it doesn't necessarily need to require object instantiation. And the only internal state for Koha::MetadataExtractor is a list of stateless objects. So I have a suggestion for an alternative using static class methods. It would be just as practical and high performance, but it would be more concise. It would look like this: $biblio_metadata->extractor->get_normalized_upc({ record => $record }) I suppose that might look a little confusing, so you could do something like 'my $extractor = $biblio_metadata->get_extractor' and then use '$extractor->get_normalized_upc({ record => $record })'. The Koha::Biblio::Metadata object has the "schema" to-hand, so the "extractor" method would dynamically return the name of the schema-specific extractor class for that metadata record, and then "get_normalized_upc" would be called as a static class method like Koha::MetadataExtractor::MARC::MARC21->get_normalized_upc({ record => $record }). It's very concise and has all the same benefits of your current implementation. I'll include a little proof-of-concept as a comment. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.