Tech Guidelines: factories vs new keyword
#3982
Conversation
|
An admin must run tests on this PR before it can be merged. |
dshevtsov
added
the
Major Update
| @@ -98,6 +100,11 @@ class Config | |||
| {% endcollapsible %} | |||
| --- | |||
|
|
|||
| 2.2.2. Factories SHOULD be used for object instantiation instead of `new` keyword. An object SHOULD be replaceable for testing or extensibility purposes. | |||
| Exception: DTOs. There is no behavior in DTOs, so there is no reason for its replaceability. | |||
There was a problem hiding this comment.
I would suggest it would be a good idea to either have an explanation for the term DTO here or link through to one.
- added reference to DTO definition
| 2.2.2. Factories SHOULD be used for object instantiation instead of `new` keyword. An object SHOULD be replaceable for testing or extensibility purposes. | ||
| Exception: [DTOs](https://en.wikipedia.org/wiki/Data_transfer_object). There is no behavior in DTOs, so there is no reason for its replaceability. | ||
| Tests can create real DTOs for stubs. | ||
| Data interfaces, Exceptions and `Zend_Db_Expr` are examples of DTOs. |
There was a problem hiding this comment.
Would be great to add code samples where we can't test or extend some class and how using factories will help to solve this issue
There was a problem hiding this comment.
Maybe having a blog post would be helpful? I'm afraid adding more explanations here would make the document grow and hard to read.
There was a problem hiding this comment.
Maybe having a blog post would be helpful? I'm afraid adding more explanations here would make the document grow and hard to read.
Good idea + add link to that post there
|
Hi @buskamuza Any update on this one? |
keharper
added
the
Internal Dev
|
This one can be merged. No more feedback received and I addressed existing one. |
|
running tests |
|
Hi @buskamuza, thank you for your contribution! |
This PR is a:
Summary
Added the rule about using factories instead of
newkeyword.See also discussion at magento/magento-coding-standard#64 (comment)
Additional information
List all affected URLs
❗️ older versions of Tech Guidelines should be also updated before merging this PR.
whatsnew
Added '2.2. Object instantiation' section to Technical Guidelines.