Optimize creation from constructor

[MrMeshok]

When object gets created from constructor, there are always checks for constructor arguments in context. This eats some of performance, especially if you don't use context. When looking at the generated code, I noticed that it can be optimized a little. I didn't benchmark it a lot, but I did see around 10% more performance with this optimization

Comparisons

Argument from source

Before:

if (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')) {
    $constructarg = $source->propertyName ?? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName');
} else {
    $constructarg = $source->propertyName ?? throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance ...');
}

After:

$constructarg = $source->propertyName ?? (
    \AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')
        ? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName')
        : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance ...')
);

Argument from source with array transformer

Before:

if (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')) {
    $values = [];
    foreach ($value['propertyName'] ?? [] as $key => $value) {
        $values[$key] = $value;
    }
    $constructarg = count($values) > 0 ? $values : \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName');
} else {
    $values = [];
    foreach ($value['propertyName'] ?? [] as $key => $value) {
        $values[$key] = $value;
    }
    $constructarg = count($values) > 0 ? $values : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance');
}

After:

$values = [];
foreach ($value['propertyName'] ?? [] as $key => $value) {
    $values[$key] = $value;
}
$constructarg = count($values) > 0
    ? $values
    : (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')
        ? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName')
        : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance')
    );

Argument without source

Before and After here logically the same, I just changed it so code would be consistent with arguments from source.

Before:

if (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')) {
    $constructarg = \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName');
} else {
    throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance');
}

After:

$constructarg = \AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')
    ? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName')
    : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance');

[MrMeshok]

I also thought about removing hasConstructorArgument check because just calling getConstructorArgument would be faster, but that's technically changing this behavior:

readonly class Dto
{
    public function __construct(
        public ?string $optionalNullableString = 'default'
    ) {}
}

$data = [];
$object = $autoMapper->map($data, Dto::class, [MapperContext::CONSTRUCTOR_ARGUMENTS => [
    Dto::class => ['optionalNullableString' => null]
]]);

$object->optionalNullableString === null; // with hasConstructorArgument check
$object->optionalNullableString === 'default'; // without hasConstructorArgument check

Maybe there should be a test for this behavior🤔. I can add it to this PR

[nikophil]

Hi @MrMeshok

nice PR, very good idea!

I also thought about removing hasConstructorArgument check because just calling getConstructorArgument would be faster, but that's technically changing this behavior

won't that even create an error if you have this kind of Dto, and using null as a constructor argument?

readonly class Dto
{
    public function __construct(
        public ?string $optionalNullableString
    ) {}
}

if we want to allow null as a constructor argument, I think we need to keep the call to array_key_exists(), WDYT?

Maybe there should be a test for this behavior

this should definitely be tested, maybe in the version without the default value in the argument

[MrMeshok]

won't that even create an error if you have this kind of Dto, and using null as a constructor argument?

readonly class Dto
{
    public function __construct(
        public ?string $optionalNullableString
    ) {}
}

Just tested it, it's just creates Dto with optionalNullableString as null, because if there's no default argument and argument is nullable, AutoMapper will assign null as fallback value. So it's kinda looks like this

$constructarg = $source['propertyName'] 
    ?? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName') // returns null
    ?? null;

You can emulate that happens if hasConstructorArgument check gets removed by replacing array_key_exists by isset in method implementation

public static function hasConstructorArgument(array $context, string $class, string $key): bool
{
    return isset($context[self::CONSTRUCTOR_ARGUMENTS][$class][$key]);
    // return \array_key_exists($key, $context[self::CONSTRUCTOR_ARGUMENTS][$class] ?? []);
}

I think we need to keep the call to array_key_exists(), WDYT?

I personally think that's a very niche feature, and I had to think very hard to come up with an example where it could matter.

[MrMeshok]

I thought about it over the weekend, there is a way to use array_key_exists and isset/?? simultaneously. When creating a mapper, we can check whether the property accepts null, if it does, we use array_key_exists, if not isset/??. Thus we preserve null values where it matters and use more performant code where it doesn't. I will work on it in another PR when I have time