Handling Medium-like text editor output at backend

Question

I'm a developer who has completed almost one year of full-stack development & learning. I published my very first package lately, and I don't know is my code good or bad. So I jumped down here.

EditorJS is block based medium-like rich text editor, so here's a link to it.

The package gets the JSON input, passes to official handler then handles array of blocks. Everything works around an array of blocks. My approach to handle these blocks is collection. At the root we have a block class to handle blocks It self, at the middle collection of blocks and at the top of that main EditorJS class wrapper. For sample JSON output, you can look at tests directory.

I hope I explained it well enough. I just need a little review of my code. Don't worry It's a small package, so It's not complex. Here's the Github repository of my code; Github repository.

Block class

<?php
namespace Megasteve19\EditorJS\Block;
{
 /**
 * Base class for all block types.
 * 
 * @author megasteve19
 */
 class Block
 {
 /**
 * @var string The block's unique ID.
 */
 private string $id;
 /**
 * @var string Type of the block.
 */
 private string $type;
 /**
 * @var array Data of the block.
 */
 private array $data;
 /**
 * Constructor
 * 
 * @param array $block The block data to fill the object with.
 * @return void
 */
 public function __construct(array $block)
 {
 $this->id = uniqid('editorjs');
 $this->type = $block['type'];
 $this->data = $block['data'] ?? [];
 }
 /**
 * Get the block's unique ID.
 * 
 * @return string
 */
 public function getId()
 {
 return $this->id;
 }
 /**
 * Get the block's type.
 * 
 * @return string
 */
 public function getType()
 {
 return $this->type;
 }
 /**
 * Get the block's data.
 * 
 * @return array
 */
 public function getData()
 {
 return $this->data;
 }
 /**
 * Rescurively search for a specific key in the block's data.
 * Syntax: `$block->find('key1.key2.key3')`.
 * 
 * @param string $key The key to get the data from.
 * @return array|string The data or empty string if not found.
 */
 public function find(string $key)
 {
 $data = $this->data;
 $keys = explode('.', $key);
 foreach($keys as $key)
 {
 if(isset($data[$key]))
 $data = $data[$key];
 else
 return '';
 }
 return $data;
 }
 /**
 * Inserts a new data key into the block's data.
 * Syntax: `$block->insert('key1.key2.key3', $value)`.
 * 
 * @param string $key The key to insert.
 * @param mixed $value The value to insert.
 * @return void
 */
 public function insert(string $key, $value)
 {
 $keys = explode('.', $key);
 $data = &$this->data;
 foreach($keys as $key)
 {
 if(empty($data[$key]))
 {
 $data[$key] = [];
 }
 $data = &$data[$key];
 }
 if(empty($data))
 {
 $data = $value;
 }
 }
 /**
 * Update the block's data with a new one.
 * Syntax: `$block->update('key1.key2.key3', $newData)`.
 * 
 * @param string $key The key to replace the data.
 * @param mixed $data The new data.
 * @return void
 */
 public function update(string $key, $newData)
 {
 $data = &$this->data;
 $keys = explode('.', $key);
 foreach($keys as $key)
 {
 if(isset($data[$key]))
 $data = &$data[$key];
 else
 return;
 }
 $data = $newData;
 }
 /**
 * Remove a key from the block's data.
 * Syntax: `$block->remove('key1.key2.key3')`.
 * 
 * @param string $key The key to remove.
 * @return void
 */
 public function remove(string $key)
 {
 $keys = explode('.', $key);
 $lastKey = array_pop($keys);
 $data = &$this->data;
 foreach($keys as $key)
 {
 if(empty($data[$key]))
 {
 return;
 }
 $data = &$data[$key];
 }
 // Unset if the key exists.
 if(!empty($data[$lastKey]))
 {
 unset($data[$lastKey]);
 }
 }
 /**
 * Convert the block to an array.
 * 
 * @return array Converted block.
 */
 public function toArray()
 {
 return [ 'type' => $this->type, 'data' => $this->data ];
 }
 /**
 * Renders HTML from the block.
 * 
 * @param string $templatePath The path to the template file.
 * @return string The HTML.
 */
 public function toHTML(string $templatePath)
 {
 if(file_exists($templatePath))
 {
 ob_start();
 extract($this->data, EXTR_SKIP);
 include $templatePath;
 return ob_get_clean();
 }
 return '';
 }
 }
}

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate.
• megasteve: Thank you, sorry for all of that
• morgwai: honestly, I still find it hard to guess what this class is supposed to do. What blocks? of cheese? of chocolate? ;-) Perhaps explaining what type may be would be useful. then explain briefly what are the most common operations that your app performs on these blocks. Cheers!
• megasteve: EditorJS is block based rich text editor, what I mean by blocks is; block-level HTML tags, header, paragraph, lists and goes on.

Answer 1

Your package builds on the getBlocks() method of EditorJS. You get all blocks, which represent HTML tags and their content, and you render it as HTML output using PHP template files. The ugly code is probably hidden in these templates?

You have not explained why you would want to access the content of a block using a stringed key. What is the practical use of that?

Anyway, I have noticed several things:

In four methods; find(), insert(), update() and remove(), you use very similar ways to get at the data. Why not use a private method for this, so you don't have to repeat yourself? Something like:

private function accessData($key)
{
 $data = &$this->data;
 $pins = explode('.', $key);
 foreach($pins as $pin) {
 $data = &$data[$pin] ?? NULL;
 }
 return $data;
}

I used $pins instead of $keys, because it is weird when a single key consists out of multiple keys. It makes more sense to say that one key has multiple pins. I also use the new null coalescing operator because this is a perfect place to use it. You can now reuse this method in the four methods I mentioned before, making those a lot shorter.

Personally I am not too fond of using extract, output buffering, and include some template file, for each block you want to translate to HTML. PHP will probably do this quite efficiently, but it seems a lot of hassle, and it is not very OOP. Now that you have your blocks in PHP, why not use the power of OOP?

Instead of using include() why not use spl_autoload_register() to load template classes? For instance, when the class name, for rendering an item list, is 'ListBlock", and the PHP file in which that class is defined is called "listblock.php", you could use something like this:

function blockToHtml($className)
{
 $path = __DIR__.'/templates/'.strtolower($className).'.php';
 if (is_readable($path)) require($path);
}
spl_autoload_register('blockToHtml');

You can now convert the data to HTML in a PHP class, using something like this inside your Block class:

public function toHTML($htmlVersion = 5)
{
 $classname = $this->getType().'Block';
 $template = new {$classname}($this->getData());
 return $template->toHTML($htmlVersion);
}

There are advantages to doing it this way. As you can see I supply $htmlVersion as a parameter to the toHTML(). This is just an example, but you can do a lot more in the template class now. You could, for instance define a toXML() method. One template class could also call another template class. This code is just more flexible, and fully OOP.

I see this a lot in programmers starting with OOP. They program some parts of their code very neatly, and even implement tests, but other parts are just old school stuff while there's no need for that. Old habits die hard.

• mickmackusa: Why not write directly to $this->data instead of declaring a reference variable $data?
• KIKO Software: @mickmackusa You mean in my accessData() method? That is because the original data cannot be used multiple times if you overwrite it.
• mickmackusa: I must not be following the logic then. Is it not the same as this: 3v4l.org/TDVF9 ?
• KIKO Software: @mickmackusa: All it does is return a part of the data array given a key.
• megasteve: About templates, yes bot no :). It would be great for package but not for MVC. Other than that I'm agree with you, I was using bitwise operators for the first time so I didn't moved away to a new method. Thanks for your time!