3 ms·
In this case, the issue is that they've tried to "DRY" up things that don't actually reduce any repetition. Yet. Maybe it will in the future, then DRY it up the
by vidarh 3y ago
In this case, the issue is that they've tried to "DRY" up things that don't actually reduce any repetition. Yet. Maybe it will in the future, then DRY it up then. Secondly, DRY is meaningless if applied at units so small that you replace one invocation of a method with another invocation of a method when the arguments to the first one are obvious. E.g. consider this from the example:
def find_admin_role
Roles.find_by(name: 'admin')
end
Assuming your application consistently uses `name`, and is consistent about class names, `find_admin_role` is not saving you complexity over Roles.find_by(name: 'admin'). People familiar with Ruby ORMs and tasked with finding an admin role will go looking for finders in the Role class. They're not likely to find your method, and so are likely to write their own.
The former introduces complexity that is only warranted to me if 1) the method of finding the admin role grows more complex so it's non-obvious, 2) the method you DRY it up into is actually being used at least twice, 3) it's placed in the most appropriate class.
And it's that second where YAGNI comes in. By all means, DRY up logic. But don't do it before it actually stops you repeating yourself in a way that is actually significant.
To #3, that "find_admin_role" method really should be `Roles.find_admin` if it's allowed to exist at all - there it's at least somewhat defensible even if it is only a single line method, because others are likely to find it there, and even if it seems relatively redundant, at least there it serves to document that this is indeed the canonical name for an admin role.