plank / laravel-metable

A package for handling arbitrary data in Laravel 5 applications
MIT License
281 stars 42 forks source link

Move `getDataType` method to abstract class #89

Closed GeoSot closed 1 year ago

GeoSot commented 1 year ago

As almost all Handlers keep a naming pattern, I added an Abstraction, which provides the same name based on the existing convention

coveralls commented 1 year ago

Coverage Status

Coverage remained the same at 98.673% when pulling 429207eb08900b2727a61be61c391732864d02d7 on GeoSot:move-handlers-type-to-abstract into f576ecee1c5a9239a3eb27acdb63ef521bbc5d43 on plank:master.

GeoSot commented 1 year ago

I left the tests same, for the only reason, that they correctly check naming and types. So no breaking changes here.

As for the naming automation, I suppose will be slightly slower as it is not reflection but just passes through some functions.

note: the reason of this proposal, is the future thought of handlers supporting Casters, and following the mind, I am trying to merge functionalities under one class

frasmage commented 1 year ago

In that case, perhaps just add a base class for the new functionality that you are looking to add as part of that change, but leave the methods as is, where possible. I don't think the inference magic is worth it here.