Hi all, I’ve been writing some code to deal with HTML forms. The code consists of several classes that assist the main ‘Form’ class in making the job of creating HTML forms easier.
Right now I have hit the point where my main form class is too big, even though I already have several assisting objects dealing with their own side of things (validator, messenger, renderer, fieldInterface, field objects etc…).
The form class has a lot of accessors, which is basically what is making the code long. I have discarded the magic method option for dealing with accessors because it will reduce visibility, but also because there is generally a lot of custom logic going on in them. There is some common functionality though, like checking for types and logging incorrect usage for which I might use static classes.
What I’m trying to sort out basically is reducing mental overload when working with the main class so I’m stuck with the following dilemma (and rather than looking to produce a long metaphysical debate, I would like to look at it from a very practical standpoint because probably none of both solutions would look neat from a puritan programming mindset) :
Use inheritance where my main Form class will extend another class that contains all accessors and properties
Use a (singleton) trait that contains all my accessors
I think both solutions would work, however I have never worked with traits and I’m unaware of any potential problems I might run into; and the inheritance solution, even though it would work for what I need, I’m not sure it really makes the case for it. In the future I might be creating actual extensions that make more sense for different form usages.
I’m a bit stuck in this dilemma because I’m unaware of potential problems I might encounter as the code grows, and architecture is obviously a determining point in scalability, and this piece of code is going to keep growing.
Thank you very much for any input on this, really appreciated!
when using traits, do not use anything inside the trait that is not part of the trait (i.e. do not rely on outside code). as an example, when you create an accessor trait, put the accessor method and the property into the trait.
One advantage about traits it that they allow you to chop your code into even smaller chunks of functionality. For example, in your case, you may have a ValidatorTrait.
Another advantage is that the same trait can be used by different types of classes. In my case, my controller and view classes also use the action trait.
Finally, traits can use other traits as well. My action trait used the container aware trait to access the dependency inject container.
I think traits have opened a whole new beautiful world of code organisation for large classes in my world. Now you could argue that if the class is too large then it is not a good class, however with power comes complexity, and I think traits do the perfect job for abstracting related code chunks from a class even if they are going to be used only for one class.
Now I’m coming up with the following traits for my form class:
Accessors
Data
Field API
Validator
Logger
Messenger
Renderer
And now I might be abusing , but it definitely adds clarity
Some of the things you mentioned should be their own class instead of a trait. Like logger and validator.
If you keep the Single Responsibility Principle in mind your code will turn out to be a lot more maintainable than when you put all of it in once class.
For example, what would happen if you need a logger somewhere else? Import the trait? What if one needs to log to file and another to the syslog, how will you set that up? With a single class it’s easy. Just configure it and pass it as a collaborator to your class. With traits it’s too tightly coupled.
The thing to understand is that traits make convenient wrappers. Of course you would not actually implement logging functionality in a trait. You would still inject a logger to handle the actual logging. The trait would implement a setLogger (for the injection) and maybe a simple log method.
Can you clarify how you would use a trait as a wrapper and what benefits it would offer? You mean using a trait to create an optional dependency on the logger with the setLogger method? If you add a simple log method that depends on the logger then if you don’t inject the logger with setLogger then it won’t work - it will not be obvious to the outside code that the log method requires that you first inject the logger.
I’ve spent some time to illustrate what might be a viable example but I have not tested this code. I agree with both sides, perhaps the logger trait is a bit overkill, however if the form needed plenty of custom logging we could put that code inside a trait, otherwise for a couple of methods like in the example below it would not be even worth it. All I want in this case, I guess, is being able to use traits as a means to contextualize code within a class, in order to maintain focus and sanity while working with it and as it grows and gets more complex.
Thanks again, the input is much appreciated.
<?php
Class Logger
{
protected static $typeExceptions = array(
'string' => "Expecting String but got [type] instead: [value]"
);
public static function logError(string $message)
{
//Thinking of using debug backtrace here to build a message to capture the method and class the error came from.
$backTraceInfo = "error in [method name] from class [className]\n";
error_log($backTraceInfo.$message);
}
public static function getTypeException ($type, $value = '')
{
if(isset(self::$exceptionMessages[$type]))
{
$message = str_replace(
array('[type]','[value]'),
array(gettype($value), $value),
self::$exceptionMessages[$type]
);
return $message;
}
}
}
Trait FormLogger
{
protected function $displayLogs = 0;
protected $errorLog = array();
protected function logError ($message, $value)
{
$message = Logger::getTypeException($message, $value) ?? $message;
if($this->displayLogs)
{
$this->setMessage($message, 'error');
}
$this->errorLog[]= array(
'message' => $message,
'value' => $value,
'type' => getType($value)
);
Logger::logError($message);
}
//..
}
Trait FormMessenger
{
public function setMessage ($text, $type)
{
// Sets a message in the UI
}
public function renderMessage ($text, $type)
{
// html ...
}
// ...
}
Trait FormAccessors
{
protected $someProperty;
public function setSomeProperty ($value = '')
{
if(is_string($value))
{
$this->someProperty = $value;
}
else
{
$this->logError("string", $value);
}
return $this;
}
// ...
}
Class Form
{
Use FormLogger,
FormAccessors,
FormMessenger;
public function __construct ($params)
{
//...
}
// Top level form logic ...
}
, but it definitely adds clarity