Client generator #56
Client generator #56
Conversation
| @@ -32,7 +46,7 @@ class Config implements ConfigInterface | |||
| * @var array | |||
| */ | |||
| protected $soapOptions = [ | |||
| 'trace' => false, | |||
| 'trace' => false, | |||
| * @internal param string $xsdName | ||
| * @internal param Property[] $properties | ||
| */ | ||
| public function __construct($name, $namespace, ClientMethodMap $methods) |
There was a problem hiding this comment.
You can use strict string types here
| /** | ||
| * @var Method[] | ||
| */ | ||
| private $parameters; |
There was a problem hiding this comment.
The type is Parameter[] instead of Method[]
| * @internal param string $xsdName | ||
| * @internal param Property[] $properties | ||
| */ | ||
| public function __construct($functionString, $parameterNamespace = null) |
There was a problem hiding this comment.
strict types. Instead of null you can use an empty string
| public function __construct($functionString, $parameterNamespace = null) | ||
| { | ||
| $this->parameterNamespace = $parameterNamespace ?: ''; | ||
| $this->parameters = $this->parseParameters($functionString); |
There was a problem hiding this comment.
The parsing should not occur in the constructor.
You can create a names constructor for this or maybe it's even better to move the parsing to a separate class or to the SoapClient class. This last one makes the package more consistent.
| $properties = explode(',', $properties[1]); | ||
| foreach ($properties as $property) { | ||
| list($type) = explode(' ', $property); | ||
| $parameters[] = new Parameter($type, $this->parameterNamespace.'\\'.$type); |
There was a problem hiding this comment.
What is the parameter namespace for? The model should be as close to soap as possible. The namespace shouldn't be here I think
There was a problem hiding this comment.
This is the type namespace
| */ | ||
| public function __construct(array $methods, $parameterNamespace = null) | ||
| { | ||
| foreach ($methods as $method) { |
There was a problem hiding this comment.
It's probably better to move this logic to the names constructor instead?
| * @param string $parameterNamespace | ||
| * @return ClientMethodMap | ||
| */ | ||
| public static function fromSoapClient(SoapClient $client, $parameterNamespace = null) |
There was a problem hiding this comment.
string $parameterNamespace = ''
| * | ||
| * @package Phpro\SoapClient\Console\Command | ||
| */ | ||
| class GenerateClientCommand extends Command |
There was a problem hiding this comment.
This one looks hard on the type generator command. Maybe we can move some logic to helpers?
Maybe we can also rethink the commands a bit - since I do not think they are working 100% like they should.
There was a problem hiding this comment.
I already have so many changes, I'd prefer it if we can do this in a separate issue.
| 'return $this->call(\'%1$s\', $%1$s);', | ||
| $param->getName() | ||
| ), | ||
| 'returntype' => '\\'.$method->getParameterNamespace().'\\'.$method->getReturnType(), |
There was a problem hiding this comment.
This should be normalized. @tombevers is working on a TypeChecker in #61
There was a problem hiding this comment.
Should i wait on Tom's PR?
…ap-client into client-generator
Added documentation Added ClientMethodAssembler to the default ruleset
Ready for review