TheDirtyDozen
KubaWerłos
SoftwareHouseInnovativeTechnologies
by
KICK ME
KeepItComplicatedKeepMeEmployed
One good programmer can easily create two new jobs a year.
KICK ME
I am having trouble understanding this code.
Let's take a look.
Do you see this function? It calls these 13 other functions and has roughly 1000 lines of commented out code in it.
Ah yes, that is the Job Security Design Pattern in action!
Hundred responsibilities principle
the more the merrier
knows very much or does very much
knows very much | ⟶ | LOC, properties, methods |
does very much | ⟶ | cyclomatic complexity |
Hundred responsibilities principle
LOC | CC | ||
|---|---|---|---|
1. | giggsey/libphonenumber-for-php/src/geocoding/data/en/61.php | 61 094 | 1 |
2. | Mpdf\Mpdf | 27 506 | 7 021 |
3. | TCPDF | 24 748 | 4 415 |
4. | voku\helper\UTF8 | 14 100 | 1 498 |
5. | ScssPhp\ScssPhp\Compiler | 10 430 | 1 493 |
6. | TYPO3\CMS\Core\DataHandling\DataHandler | 10 020 | 1 450 |
7. | WordPress/wp-includes/functions.php | 8 467 | 826 |
8. | PhpOffice\PhpSpreadsheet\Reader\Xls | 8 088 | 1 020 |
Untestability
Profits of not having tests:
- not doing work no one pays for
- no problems with failing build on CI
- increase need for manual testing
- more thrilling changes to the code
- make it harder to change the code without worrying that anything will get broken
- deprive us from having documentation for what the code is actually meant to do
Untestability
open/closed principle
A module should be open for modification.
A module should be closed for extension.
Redefine the functionality
reinventing the wheel
re-implementing (limited) PHP functions
prefer implementing custom solutions over using open source library
overriding methods
Really big interfaces
big ⟶ complicated
complicated ⟶ advanced
advanced ⟶ smart
Really big interfaces
| Methods | ||
|---|---|---|
1. | Carbon\CarbonInterface | 375 |
2. | Magento\Sales\Api\Data\OrderInterface | 274 |
3. | Illuminate\Support\Enumerable | 139 |
4. | Drupal\Core\Form\FormStateInterface | 91 |
5. | Knp\Menu\ItemInterface | 58 |
6. | Doctrine\ORM\Query\TreeWalker | 50 |
7. | Laminas\Barcode\Object\ObjectInterface | 47 |
8. | Symfony\Component\Form\FormInterface | 35 |
You should rely on others
Extend third party libraries directly
+ class InvoicePdf extends FPDF
+ {
+ }Use third party libraries directly
new Symfony\Component\Process\Process(/* ... */);Hundred responsibilities principle |
Untestability |
Redefine the functionality |
Really big interfaces |
You should rely on others |
Bus factor
Bus factor
Bus factor
Bus factor
Bus factor
The number of team members who,
if run over by a bus,
would put the project in jeopardy.
Our goal is to decrease the bus factor.
Early Optimization
Lack of optimization is the root of all evil.
It is not difficult to know what will be the bottleneck.
Prefer efficiency over simplicity.
Try to cache everything you can.
Inheritance over composition
Magento\Backend\Block\System\Store\Edit\Form\Storeextends Magento\Backend\Block\System\Store\Edit\AbstractFormextends Magento\Backend\Block\Widget\Form\Genericextends Magento\Backend\Block\Widget\Formextends Magento\Backend\Block\Widgetextends Magento\Backend\Block\Templateextends Magento\Framework\View\Element\Templateextends Magento\Framework\View\Element\AbstractBlockextends Magento\Framework\DataObjectInheritance over composition
+ public function __construct(
+ \Magento\Framework\Model\Context $context,
+ \Magento\Framework\Registry $registry,
+ \Magento\Framework\Api\ExtensionAttributesFactory $extensionFactory,
+ AttributeValueFactory $customAttributeFactory,
+ \Magento\Framework\Stdlib\DateTime\TimezoneInterface $timezone,
+ \Magento\Store\Model\StoreManagerInterface $storeManager,
+ \Magento\Sales\Model\Order\Config $orderConfig,
+ \Magento\Catalog\Api\ProductRepositoryInterface $productRepository,
+ \Magento\Sales\Model\ResourceModel\Order\Item\CollectionFactory $orderItemCollectionFactory,
+ \Magento\Catalog\Model\Product\Visibility $productVisibility,
+ \Magento\Sales\Api\InvoiceManagementInterface $invoiceManagement,
+ \Magento\Directory\Model\CurrencyFactory $currencyFactory,
+ \Magento\Eav\Model\Config $eavConfig,
+ \Magento\Sales\Model\Order\Status\HistoryFactory $orderHistoryFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Address\CollectionFactory $addressCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Payment\CollectionFactory $paymentCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Status\History\CollectionFactory $historyCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Invoice\CollectionFactory $invoiceCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Shipment\CollectionFactory $shipmentCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Creditmemo\CollectionFactory $memoCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\Shipment\Track\CollectionFactory $trackCollectionFactory,
+ \Magento\Sales\Model\ResourceModel\Order\CollectionFactory $salesOrderCollectionFactory,
+ PriceCurrencyInterface $priceCurrency,
+ \Magento\Catalog\Model\ResourceModel\Product\CollectionFactory $productListFactory,
+ \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
+ \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
+ array $data = [],
+ ResolverInterface $localeResolver = null,
+ ProductOption $productOption = null,
+ OrderItemRepositoryInterface $itemRepository = null,
+ SearchCriteriaBuilder $searchCriteriaBuilder = null,
+ ScopeConfigInterface $scopeConfig = null,
+ RegionFactory $regionFactory = null,
+ RegionResource $regionResource = null,
+ StatusLabel $statusLabel = null
+ ) {
+ WET
WeEnjoyTyping
Weeks of programming can save you hours of planning
Weeks of programming can save you hours of planning
meetings = waste of time
Naming
Hungarian notation:
+ $bCorrect;
+ $iCount;
+ $sName;
+ Commonly known names:
+ class DatabaseUtil {}
+ class UserHelper {}
+ class PasswordManager {}
+ class AccountFunctions {}
+ Naming
Use short names and describe more in comment:
+ /*
+ * This function returns random item
+ * for matching name and category
+ */
+ function get2($name, $category) { /* ...*/ }
+ Do abbreviate:
+ $w; // this is username
+ Magic numbers and strings
+ class PasswordManager
+ {
+ public function setPassword($password) {
+ if (strlen($password) < 8) {
+ throw new Exception('Password must be longer than 8 characters!');
+ }
+ /* ... */
+ }
+ }
+ Magic numbers and strings
Anemic domain model
Contains little or no business logic
(validations, calculations, business rules etc.).
- You can claim it's a domain model and brag to your developer friends and put it on your resume.
- It's easy to generate automagically from database tables.
- It maps to Data Transfer Objects surprisingly well.
Anemic domain model
The fundamental horror of this anti-pattern is that it's so contrary to the basic idea of object-oriented design.
Gypsy wagon
(Poltergeist)
(Big DoIt Controller Class)
When we anticipate the need for a more complex architecture.
Limited responsibilities and roles.
Effective life cycle is short.
Improbability factor
A problem exists in a system,
and we are not going to fix it,
because chances of the problem ever surfacing are small.
Cargo cult programming
(Stack Overflow Driven Development)
We are ready!
| KICK ME | ⟶ | KISS |
Hundred responsibilities principle | ⟶ | Single responsibility principle |
Untestability | ⟶ | Open–closed principle |
Redefine the functionality | ⟶ | Liskov substitution principle |
Really big interfaces | ⟶ | Interface segregation principle |
You should rely on others | ⟶ | Dependency inversion principle |
Bus factor | ⟶ | Increasing the bus factor |
Early Optimization | ⟶ | Forget about small efficiencies |
Inheritance over composition | ⟶ | Composition over inheritance |
WET | ⟶ | DRY |
Weeks of programming can save you hours of planning | ⟶ | Hours of planning can save you weeks of programming |
Naming | ⟶ | Descriptive, meaningful naming |
Magic numbers and strings | ⟶ | Named constants |
Anemic domain model | ⟶ | Rich Domain Model |
Gypsy wagon | ⟶ | Single responsibility principle |
Improbability factor | ⟶ | Test-first approach |
Cargo cult programming | ⟶ | Cargo cult used responsibly |
One good programmer can easily create two new jobs a year.
One bad programmer can easily create two new jobs a year.
Lack of optimization is the root of all evil.
Premature optimization is the root of all evil.
A module should be open for modification.
A module will be said to be open if it is still available for extension.
A module should be closed for extension.
A module will be said to be closed if it is available for use by other modules.
Sources
- https://en.wikipedia.org/wiki/The_Dirty_Dozen
- https://mrrobot.fandom.com/wiki/E_Corp
- https://www.linkedin.com/pulse/job-security-design-pattern-jsdp-mike-veigel
- https://sahandsaba.com/nine-anti-patterns-every-programmer-should-be-aware-of-with-examples.html
- https://sourcemaking.com/antipatterns/software-project-management-antipatterns
- https://martinfowler.com/bliki/AnemicDomainModel.html
- https://stackoverflow.com/questions/258534/anemic-domain-model-pros-cons
- https://www.promptworks.com/blog/in-defense-of-cargo-cult-programming
Questions?
Thanks
kubawerlos