Replace node type in Drupal

Question

In Drupal 11 or greater, this code is used to change the node type of a node. How would you improve the code if at all?

use Drupal\node\Entity\Node;
use Drupal\Core\Logger\LoggerChannelInterface;
$nid = NID_COMES_HERE;
if (!empty($nid) && is_numeric($nid)) {
 try {
 $node = Node::load($nid);
 if ($node) {
 if ($node->getType() == 'EXISTING_NODE_TYPE_COMES_HERE') {
 $node->set('type', 'NEW_NODE_TYPE_COMES_HERE');
 $node->save();
 } else {
 \Drupal::logger('my_change_node_type_module')->error('Node type mismatch for node ID: @nid', ['@nid' => $nid]);
 }
 } else {
 \Drupal::logger('my_change_node_type_module')->error('Node not found for ID: @nid', ['@nid' => $nid]);
 }
 } catch (\Exception $e) {
 \Drupal::logger('my_change_node_type_module')->error('Error loading node with ID @nid: @message', ['@nid' => $nid, '@message' => $e->getMessage()]);
 }
}

Answer 1

Consider following a standard guide

Many PHP developers follow the PSR standards - e.g. PSR-12 which contains a section about lines:

2.3 Lines There MUST NOT be a hard limit on line length.

The soft limit on line length MUST be 120 characters.

Lines SHOULD NOT be longer than 80 characters; lines longer than that SHOULD be split into multiple subsequent lines of no more than 80 characters each.

Some of those long lines can be split to multiple lines - e.g.

 } else {
 \Drupal::logger('my_change_node_type_module')->error('Node type mismatch for node ID: @nid', ['@nid' => $nid]);

Can be updated to this, which requires no scrolling over if the window is narrow:

 } else {
 \Drupal::logger('my_change_node_type_module')
 ->error('Node type mismatch for node ID: @nid', ['@nid' => $nid]);

Consider using strict equality checks

If $node->getType() always returns a string then the strict equality operator === can be used instead of the loose equality operator ==. It is a good habit to use strict equality operators whenever possible as it requires no type juggling.

Consider returning early to decrease indentation levels

There are three levels of nested conditionals and the lines within the blocks are quite long. One could reverse the logic a bit to "return early" (or exit if not in a function/method) which can decrease indentation levels. For example:

if (empty($nid) || !is_numeric($nid)) {
 exit;
}
try {
 $node = Node::load($nid);
 if (!$node) {
 \Drupal::logger('my_change_node_type_module')
 ->error('Node not found for ID: @nid', ['@nid' => $nid]);
 exit;
 }
 if ($node->getType() !== 'EXISTING_NODE_TYPE_COMES_HERE') {
 \Drupal::logger('my_change_node_type_module')
 ->error('Node type mismatch for node ID: @nid', ['@nid' => $nid]);
 exit;
 }
 $node->set('type', 'NEW_NODE_TYPE_COMES_HERE');
 $node->save();
} catch (\Exception $e) {
 \Drupal::logger('my_change_node_type_module')
 ->error('Error loading node with ID @nid: @message', ['@nid' => $nid, '@message' => $e->getMessage()]);
 }
}

Obviously it requires additional lines to call exit; however the indentation levels are minimal and some may find it simpler to read.

If there is a desire not to utilize exit then the else keyword could be used:

if (!empty($nid) && is_numeric($nid)) {
 try {
 $node = Node::load($nid);
 
 if (!$node) {
 \Drupal::logger('my_change_node_type_module')
 ->error('Node not found for ID: @nid', ['@nid' => $nid]);
 } elseif ($node->getType() !== 'EXISTING_NODE_TYPE_COMES_HERE') {
 \Drupal::logger('my_change_node_type_module')
 ->error('Node type mismatch for node ID: @nid', ['@nid' => $nid]);
 } else {
 $node->set('type', 'NEW_NODE_TYPE_COMES_HERE');
 $node->save();
 }
 } catch (\Exception $e) {
 \Drupal::logger('my_change_node_type_module')
 ->error('Error loading node with ID @nid: @message', ['@nid' => $nid, '@message' => $e->getMessage()]);
 }
 }

• the_humble_asker: You probably meant that if ($node->getType() == 'EXISTING_NODE_TYPE_COMES_HERE' should have === instead of !== because I only do the operation if I match the existing the node type.
• Sᴀᴍ Onᴇᴌᴀ: "I only do the operation if I match the existing the node type" - yes that is why the condition $node->getType() !== 'EXISTING_NODE_TYPE_COMES_HERE' leads to the call to \Drupal::logger(...)->error(...) followed by exit to avoid the subsequent lines where the set() and save() methods are called on $node.
• the_humble_asker: If the existing node type isn't matched then exit. Okay, I think I get it now. These exit statements are not something I am used to from Bash and JavaScript, it's a bit confusing to me. If I could I wonuldn't even change the node type from PHP (laughing), so given that the exit statements are not obligatory (due to to the try-catch block I assume), I may just pass on them. Anyway thanks a lot Sam, and I have gladly accepted your answer, given generously.
• the_humble_asker: I have implemented all changes besides the exit statements and of course changed == to === as well.
• the_humble_asker: Sam, may I further add that I never liked return in JavaScript and almost always found it redundant and preferred a longer clearer code; if exit in PHP resembles return in JS, than yeah, I'll pass on it. You may want to edit and insert a sentence about any such resemblence.