How NOT to write (OOP PHP) code

My comment on one of the files of SuiteCRM:

https://github.com/salesagility/SuiteCRM/issues/3561

which was closed and kindly “put” aside (IGNORED basically!):

https://trello.com/b/Ht7LbMqw/suitecrm-suggestion-box

So…

Expected Behaviour

It has to extend/implement only SugarBean and some relevant functions.
Many functions must be implemented by separate classes
esp. email management functions related to POP3 and IMAP
so that we can maintain and extend the features easily.
e.g. replace IMAP layer with a HTTP API
We must aim for smaller functions and classes:

http://swreflections.blogspot.co.uk/2012/12/rule-of-30-when-is-method-class-or.html

Actual Behaviour

https://github.com/salesagility/SuiteCRM/blob/master/modules/InboundEmail/InboundEmail.php

Includes/implements so much code.
here is a short list of almost 170 functions

– InboundEmail::getFormattedRawSource()
– InboundEmail::convertToUtf8()
– InboundEmail::getFormattedHeaders()
– InboundEmail::emptyTrash()
– InboundEmail::importMessages()
– InboundEmail::checkEmail()
– InboundEmail::fetchCheckedEmails()
– InboundEmail::search()
– InboundEmail::retrieve*()
– InboundEmail::save*()
– InboundEmail::*IMAP*() many IMAP functions
– InboundEmail::*Pop3*() many POP3 functions
– InboundEmail::*Cache*() many Cache functions
– InboundEmail::*Mailserver() connect/disconnect etc.
– InboundEmail::*Folder() rename, delete etc.

etc. etc. !!! You get the idea 🙁

Also, it is polluted with a function and two classes:

– function this_callback
– class temp
– class Overview

Possible Fix

Many functions must be implemented by separate classes
e.g. ImapManager, Pop3Manager, EmailParser, CacheManager, FileSystemObject, etc.

My Environment

* SuiteCRM Version used: 7.8.3
* Browser name and version (e.g. Chrome Version 58 (64-bit)):
* Environment name and version (e.g. MySQL 5.7, PHP 7.0):
* Operating System and version (e.g Ubuntu 16.04)

Leave a Comment