3
\$\begingroup\$

I've been using PHP labels in my Controllers for code folding. PHP labels are actually used with goto statements. But I'm using labels only because it's so much easier to fold my code in different IDEs using labels + curly braces, and I can use labels as titles for different sections of the code.

Here's an example. Take a look at SettingCartFields label.

<?php

namespace App\Http\Controllers;

use App\Cart;
use Illuminate\Http\Request;
use App\Http\Controllers\Controller;

class CartController extends Controller
{
    public function create(Request $request)
    {
        $cart = new Cart;

        SettingCartFields: {
            $cart->field1 = $request->field1;
            $cart->field2 = $request->field2;
            $cart->field3 = $request->field3;
            $cart->field4 = $request->field4;
            $cart->field5 = $request->field5;
            $cart->field6 = $request->field6;
        }

        $cart->save();
    }
}

I have a few questions, are there any performance issues with this? Am I introducing some kind of overhead by adding a lot of labels in my code? Is there any better way to do this?

\$\endgroup\$
1
  • \$\begingroup\$ Unused labels might result in confusion for others, if you do so in shared repositories. If all you want to do is to collapse the code block, then consider placing the code blocks in functions of their own as Nigel suggested. \$\endgroup\$ – alamoot Dec 17 '19 at 20:00
4
\$\begingroup\$

The main issue I have with your approach is that it is non-standard and purely for the way that you work. Others looking at your code may be a bit confused as to what you are doing and why. In your example, there is (IMHO) little benefit in being able to fold the code as there is not really enough code to need to fold the code up, but in larger code segments there may be a use for it.

For me - it looks as though you are in the initial stages of identifying logically grouped functionality within your code, something that could then be extended by extracting these logical groups into new class methods.

The exact approach I would take depends on how much other processing you have with the cart either pre or post this main chunk of code.

If there is a lot of other processing around what the cart does, then you could just pass the cart and the request to a new method and get something like...

class CartController extends Controller
{
    public function create(Request $request)
    {
        $cart = new Cart;

        $this->setCartFields ( $cart, $request );

        $cart->save();
    }

    private function setCartFields ( Cart $cart, Request $request ) {
        $cart->field1 = $request->field1;
        $cart->field2 = $request->field2;
        $cart->field3 = $request->field3;
        $cart->field4 = $request->field4;
        $cart->field5 = $request->field5;
        $cart->field6 = $request->field6;
    }
}

If the data from the request forms the basis of the data for the cart, then this could instead create it's own cart, initialise the data and return the newly created cart for further processing...

class CartController extends Controller
{
    public function create(Request $request)
    {

        $cart = createCart ( $request );

        $cart->save();
    }

    private function createCart ( Request $request ) : Cart {
        $cart = new Cart;
        $cart->field1 = $request->field1;
        $cart->field2 = $request->field2;
        $cart->field3 = $request->field3;
        $cart->field4 = $request->field4;
        $cart->field5 = $request->field5;
        $cart->field6 = $request->field6;

        return $cart;
    }

}

The problem being that you could eventually end up with all of the processing in the newly created method and rather than relieve the problem you have just moved it. This is only something you can decide on a per instance basis.

\$\endgroup\$
3
\$\begingroup\$

Most has already been said by Nigel Ren. However, your questions about performance issues and overhead remain unanswered.

Using a label comes with very little overhead, so you really don't need to worry that it will impact the performance of your code.

PHP first parses your code into tokens and then compiles these into opcodes. These opcodes are stored in memory and executed. Since your labels don't actually do anything, they don't require memory storage or execution time.

That being said: I really dislike what you do with these labels. If I didn't know why you used them, I would be wondering why they are there. If I had to work on your code I would have the additional worry that any of the labels might be used somewhere in your code. So if I wanted to remove one, because it doesn't seem to serve any purpose, I would have to first check all of the code for references to this label. I would therefore strongly recommend to use language elements only for their intended purpose.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you. I see what you mean. \$\endgroup\$ – Mukarram Khalid Dec 19 '19 at 11:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.