commercetools-jvm-sdk icon indicating copy to clipboard operation
commercetools-jvm-sdk copied to clipboard

In AttributeDefinitionBuilder "attributeType" and "of" members look inconsistent compared to other builders

Open andrii-kovalenko-ct opened this issue 8 years ago • 2 comments

  • AttributeDefinitionBuilder#of() method doesn't have an implementation which accepts AttributeDefinition instance (kind of "clone" implementation)

  • AttributeDefinitionBuilder#of() has only one implementation with mandatory name, label and attributeType arguments. Are all of these properties mandatory from the scratch? The name and label might be set using setters later, the attributeType - no. And to reuse a builder i always must write name and label to reach third attributeType argument, even if name or label is not matter on current builder creation step.

  • AttributeDefinitionBuilder#attributeType is final. If it really should be unchangeable, we should at least to have an #of(AttributeType attributeType) implementation.

So, the current methods set looks a bit inconsistent with other platform entities builders implementations, like ProductDraftBuilder, ProductVariantDraftBuilder and some other.

andrii-kovalenko-ct avatar Feb 24 '17 17:02 andrii-kovalenko-ct

Thanks for reporting these inconsistencies. We're currently working on improving our code generator to make the factory methods more consistent (see #1321) and as soon as we have finished that we will have a closer look at your issue. So stay tuned :-)

katmatt avatar Feb 27 '17 17:02 katmatt

Right now our AttributeDefinition interface is used for creating new attribute definitions. But according to our API documentation, attribute definitions should be created with an AttributeDefinitionDraft. But the AttributeDefinitionDraft was introduced in our API after we added the initial support for attribute definitions to our SDK.

Introducing AttributeDefinitionDraft will make our SDK more consistent with our API specification and the automatically generated AttributeDefinitionDraftBuilder will then provide consistent of(...) factory methods out of the box.

We will review the impact of this change and see when we can fix this inconsistencies.

katmatt avatar Mar 08 '17 10:03 katmatt