User-defined roles in releases

[darrell-k]

I came across this when analysing another bug: the user has a custom role tag which contains "ARTIST" in its name, which meant the tag was being ignored in Slim::Menu::BrowseLibrary::Releases as it triggered a next in the loop once the standard ARTIST tag was processed.

This change separates out standard from user-defined tags so that they can both be processed correctly.

[darrell-k]

Like this?

[michaelherger]

Like this?

Much more readable, IMHO, yes. But I did that suggestion probably too late at night... It's still a grep in a loop which usually is a lot of work. Now if you look at what we're actually doing, you'll find:

• splitDefaultAndCustomRoles is calling defaultContributorRoles
• defaultContributorRoles does a grep over contributorRoles
• in that grep we do a typeToRole()
• typeToRole() is doing a relatively cheap hash lookup
• defaultContributorRoles thus converts all contributorRoles to numbers for comparison, but returns the strings again to the caller
• the caller (splitDefaultAndCustomRoles) then loops over its own list of strings as split from $roles to compare them again one by one as strings

Wouldn't it be much easier to have a function sub isDefaultContributorRole($role) { typeToRole($role) < MIN_CUSTOM_ROLE_ID } which could be used in defaultContributorRoles as well as in splitDefaultAndCustomRoles? You'd only have one iteration over the role list, the rest would be cheap hash lookups. No need to get the default contributor roles (which does another iteration over a set of values). And you'd save another helper variable (@userDefinedRoles).