-1

I'm trying to share a value of $shippingMethod from the getMainDetails() function to the estimateDeliveryDate() function in the same class to run a conditional but the second function doesn't appear to be getting the value:

private function getMainDetails($order)
{
    //This value is correctly being set from my order details in Magento
    $shippingMethod = strtolower($order->getShippingDescription());
}

private function estimateDeliveryDate($orderCreatedDate)
{
    if ($shippingMethod == 'saturday delivery') {
        echo 'Saturday Delivery';
    } else {
        echo 'Standard Delivery';
    } 
}

Wondered if someone could help?

Thank you.

Improve this question
5
  • 1
    The best way is probably to have getMainDetails(...) return $shippingMethod, and then pass it as an argument to estimateDeliveryDate(...) Commented Sep 1, 2017 at 14:04
  • php.net/manual/de/language.variables.scope.php There you'll find all you need for that. Let me know if you need some more help Commented Sep 1, 2017 at 14:11
  • The problem is that your estimateDeliverDate method doesn't define the $shippingMethod variable. If you save it to the object, other methods can use it. $this->shippingMethod = strotolower(...);, then inside your other functions do $this->shippingMethod == 'saturday delivery' Commented Sep 1, 2017 at 14:13
  • stackoverflow.com/a/1699124/8188398 That would be your solution Commented Sep 1, 2017 at 14:15
  • 1
    Possible duplicate of Access variable from scope of another function? Commented Sep 1, 2017 at 14:16

2 Answers 2

Reset to default
2

Use properties of the class ($this->variable), instead of local variables ($variable). You also have a syntax error in your code, where you compare for the value of $shippingMethod.

The method below assumes that you call getMainDetails() before being able to use estimateDeliveryDate(). You should however ensure that, by having $order being passed to estimateDeliveryDate(), and call getMainDetails() from there (where you return from the getter instead of set a property).

class Foo (
    private $this->shippingMethod; // Initialize

    /* Your other methods */

    private function getMainDetails($order)
    {
        //This value is correctly being set from my order details in Magento
        $this->shippingMethod = strtolower($order->getShippingDescription());
    }

    private function estimateDeliveryDate($orderCreatedDate)
    {
        if ($this->shippingMethod == 'saturday delivery') {
            echo 'Saturday Delivery';
        } else {
            echo 'Standard Delivery';
        }
    }
)

Alternatively, you can return from each function - which is what you should do from methods that is a "getter", i.e. getMainDetails()

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Yes, but... how do you ensure that getMainDetails is called before estimateDeliveryDate is, i.e. that the state is consistent?
1

You need to add your variable as a property, like so:

class myClass
{
    private $shippingMethod;

    private function getMainDetails($order)
    {
        //This value is correctly being set from my order details in Magento
        $this->shippingMethod = strtolower($order->getShippingDescription());
    }


    private function estimateDeliveryDate($orderCreatedDate)
    {
        if ($this->shippingMethod == 'saturday delivery')
        {
            echo 'Saturday Delivery';
        } else {
            echo 'Standard Delivery';
        }

    }
}

EDIT

However, a more solid approach at this would be something along these lines:

class DeliveryHandler
{
    private $order;

    public function __construct(Order $order)
    {
        $this->order = $order;
    }

    private function getDeliveryDateEstimate()
    {
        if ($this->order->getShippingMethod() == 'saturday delivery') {
            return 'Saturday Delivery';
        } else {
            return 'Standard Delivery';
        }

    }
}

class Order
{
    public function getShippingMethod()
    {
        return strtolower($this->getShippingDescription());
    }
}

Few of things going on in that example.

  1. I moved shippingMethod() into the Order class as it is not the DeliveryHandlers responsibility so it should not have to care about what's going on in that method. The data belongs to and comes from Order.

  2. I made getDeliveryDateEstimate() return a string instead of using echo. This makes your code more reusable - eg what if one day you want to pass this to a template or another variable instead of echoing it. This way you keep your options open.

  3. I'm using dependency injection to pass the Order class into the DeliveryHandler, thus making the public interface of Order available to DeliveryHandler.

If you happen to have a laracast subscription you could check out these courses here, they explain all this stuff in a nice digestible format:

https://laracasts.com/series/object-oriented-bootcamp-in-php

https://laracasts.com/series/solid-principles-in-php

Improve this answer

4 Comments

A function that's called get... but doesn't return anything and instead sets a property is severely misnamed and/or misguided.
true @deceze but I didn't choose these names, I merely copied the author's code
@deceze There is a lot more code in the functions that sets/gets but I didn't want to paste all of that here as i didn't feel it was relevant.
@doubleplusgood - I extended my answer to highlight how you could implement this in a more oop friendly fashion

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.