 |
@DenisChenu Feel free to leave feedback. Current discussion about how to increase testability of the code-base, in this case our models. |
 |
Why not , but i need to learn clean difference between services and dao … a sample is OK :) |
|
|
E.g., if you have a method in a class with no "this" and no "self" (and it's static even), one should consider moving it to a service class. What's the difference between a helper function and a service class? A service class can be *mocked* in a test, meaning that if class A depend on service class B, we can create a dummy class B_dummy when testing A. This is not possible for helper functions (although a function is easier to test than a stateful class). |
|
|
We have a lot of Service in current models ? Just some definition ? No ? |
|
|
Consider copyToCentral method: https://github.com/LimeSurvey/LimeSurvey/blob/master/application/controllers/admin/participantsaction.php#L2307 It is thin, which is good. Basically just dealing with the outside world (request) and delegating work to another class. BUT: That class is an active record model, which is bullshit. Active records should ONLY deal with database input/output. What we need is a layer between models and controllers which can more easily be tested than a controller action. So this method here should be moved to a CPDBCopyService class: https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/Participant.php#L1941 |
|
|
So instead of "thin controllers, fat models" pattern, it should be "thin controllers, thin models, thin services". :) All our helper functions could be moved into services, too. But that's another topic. |
|
|
Right for copyToCentral … (me never really work with CPDB …) And better understand here :) And [Survey::replacePolicyLink](https://github.com/LimeSurvey/LimeSurvey/blob/c8c8ef6736f604718ec84e91a558683cd771b07f/application/models/Survey.php#L1951) where is the best place ? |
|
|
New class SurveyPolicyService Btw, "dao" folder should maybe be called "activerecords". Service can inherit from Yii CModel class (not sure what it does, honestly). |
|
|
> Service can inherit from Yii CModel class Or maybe not, since a service doesn't hold so much data (state). A third folder called "forms" or "formmodels" would also be good. |
|
|
> A third folder called "forms" or "formmodels" would also be good. Yep :) |
|
|
Then another possibility: ``` models/ forms/ InstallerForm... activerecords/ User... services/ SurveyPolicyService ``` Putting services outside of model folder since they don't inherit from CModel. |
- Anonymous
