Cannot set typehinted properties to initial null

[YOzaz]

This doesn't work:

/** @var \Google\AdsApi\AdManager\v201808\InventoryTargeting $inventory_targeting **/
$inventory_targeting->setTargetedAdUnits(null);

Because it's typehinted as an array:
https://github.com/googleads/googleads-php-lib/blob/master/src/Google/AdsApi/AdManager/v201808/InventoryTargeting.php#L51

Suggestions:

• Provide default value as per http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration - "The declaration can be made to accept NULL values if the default value of the parameter is set to NULL": public function setTargetedAdUnits(array $targetedAdUnits = null), or
• Provide additional helper functions like public function unsetTargetedAdUnits().

[thangduo]

Hi Marijus Plančiūnas,

Could you please explain where you found the code below?

$inventory_targeting->setTargetedAdUnits(null);

Or, are you suggesting a feature for unsetting targeted ad units?

Thanks,
Thang Duong

[YOzaz]

Hi @thangduo,

Sorry, yes - it's a feature request, not necessarily for targetedAdUnits - but for all setters at entity classes in general. At the moment, if property is set, and is a type of array / collection, there is no way to unset it.

As there was an upgrade pushed for setting general properties to null (#426, https://github.com/googleads/googleads-php-lib/releases/tag/33.0.0), it would be logical to upgrade structured properties to have this "feature" as well.

Typehinted function arguments forbids it at the moment.

Thanks!

[thangduo]

Hi Marijus,

Thanks for making the feature request. We will consider it for a future release.

We understand that this feature request would apply for a larger scale than just the targetedAdUnits class. Could you please share your usecase of how you would instantiate the objects? Are you loading it from a storage (string, database, remote service)? And why would you want to reset the fields / properties after they were set?

Thanks,
Thang Duong

[YOzaz]

Hi again @thangduo,

The real-world scenario would be modifying already existant line item targetings.
Let's say, we query line item through LineItemService, and we get it as a LineItem object, which is targeted to specific ad units. If we'd want to target a placement instead of direct ad units, therefore we need to unset / remove current ad unit targeting. And it's not possible - setting targetedAdUnits to null can't be done, and if we set it to an empty array - SOAP throws NOT_NULL error.
What we were doing before was copying all properties to new LineItem object, but as they are envolving - we've switched to ReflectionClass solution with making required LineItem properties accessible and making them null. But that's more a hack than a solution...
Current GoogleAds PHP Lib filters out null properties before SOAP request, so it would work fine. As an alternative would be filtering out empty arrays as well, but I'm not sure that wouldn't be a breaking change and have implications for other apps.

Br,
Marijus

[thangduo]

Hi Marijus,

Thanks for providing more details about this feature request. Currently this feature is prioritized as P2 and will be considered in future releases of this client lib.

I will leave this feature request open for now.

For other readers of this thread, if this feature request is important for you, please add your comments regarding your use case.

Thanks,
Thang Duong

[YOzaz]

Thank you Thang.

[fiboknacky]

Fixed with this release.