ctrlo / Dancer2-Plugin-Auth-Extensible-Provider-DBIC

Dancer2::Plugin::Auth::Extensible::Provider::DBIC Perl module
Other
2 stars 4 forks source link

Use ResultSet::single() or ::first() rather than ::all()? #21

Open cxw42 opened 4 years ago

cxw42 commented 4 years ago

Thank you for this module, which is working well for me! Looking at the source, I noticed that the user search is generally my ($user) = ...->all():

https://github.com/ctrlo/Dancer2-Plugin-Auth-Extensible-Provider-DBIC/blob/5296ad3c020960e14a8dd8e497358e90006b2ee6/lib/Dancer2/Plugin/Auth/Extensible/Provider/DBIC.pm#L603

https://github.com/ctrlo/Dancer2-Plugin-Auth-Extensible-Provider-DBIC/blob/5296ad3c020960e14a8dd8e497358e90006b2ee6/lib/Dancer2/Plugin/Auth/Extensible/Provider/DBIC.pm#L667

https://github.com/ctrlo/Dancer2-Plugin-Auth-Extensible-Provider-DBIC/blob/5296ad3c020960e14a8dd8e497358e90006b2ee6/lib/Dancer2/Plugin/Auth/Extensible/Provider/DBIC.pm#L690

https://github.com/ctrlo/Dancer2-Plugin-Auth-Extensible-Provider-DBIC/blob/5296ad3c020960e14a8dd8e497358e90006b2ee6/lib/Dancer2/Plugin/Auth/Extensible/Provider/DBIC.pm#L718

https://github.com/ctrlo/Dancer2-Plugin-Auth-Extensible-Provider-DBIC/blob/5296ad3c020960e14a8dd8e497358e90006b2ee6/lib/Dancer2/Plugin/Auth/Extensible/Provider/DBIC.pm#L802

DBIx::Class::ResultSet::single() claims to avoid creating a cursor if there's only one result. Since we only keep one result anyway, would it be worth using single() for speed? If not, would you please let me know the details for my own understanding?

Alternatively, would my $user = ...->first improve code clarity?

Thanks for considering this request!

racke commented 4 years ago

It would certainly improve the readability of the code. The query should only yield one result, otherwise your database structure / data is somewhat questionable. @cxw42 thanks for the hint.