GraphQlQueryParameter<T> and QueryBuilderParameter<T> design questions

[gao-artur]

Hi. I have a few questions regarding GraphQlQueryParameter<T> and QueryBuilderParameter<T> design. The first one is used to create the parametrized query (WithParameter method). The second is to provide an inline arguments value or parameter name (With{FIELD} methods).

1. GraphQlQueryParameter<T> has a few constructors. When using

   public GraphQlQueryParameter(string name, string graphQlTypeName = null)

   I expect the nullability to be inferred from the T nullability, including respecting the NRT if enabled. But instead, it always creates nullable arguments. For example, Int instead of Int!. It also seems to have a bug in the following line because ? shouldn't be appended to the graphql type name:

   var nullableSuffix = nullableUnderlyingType == null ? null : "?";

So, my question is if there is any reason why this constructor won't be able to infer the nullability from the T?

2. There is an additional constructor.

   public GraphQlQueryParameter(string name, T defaultValue, bool isNullable = true)

   That allows to define the nullability manually, but T defaultValue is required in this constructor. Any reason why I can't override the nullability without specifying the default value?

3. The QueryBuilderParameter<T> doesn't have public constructors. This forces me to use GraphQlQueryParameter<T> and provide redundant parameters like graphql type name or isNullable and enforces validations that are not relevant here. I understand that I can reuse an instance that was passed into WithParameter but it makes the API "less fluent". I also understand that adding a public constructor with a single argument for a parameter name will create ambiguity with a private constructor when T is string, but maybe design can be changed slightly to allow simple creation of the QueryBuilderParameter<T> instance.

[Husqvik]

one problem with the nullability is that it's not only specified by the type itself. You can pass the value hard coded in the query string as within the parameter object so null value can be passed to parameter that is defined as not null.

public GraphQlQueryParameter(string name, string graphQlTypeName, T defaultValue) probably should be market obsolete, it comes from the time where there wasn't complete bidirectional mapping between the GQL types and the generated .NET types. Hopefully that is now complete so it shouldn't be any reason to pass graphQlTypeName manually.

[gao-artur]

Yeah, I understand that user will always find a way to misuse the library :) But I think, providing defaults that will hit most of the time is better than just make everything nullable by default. And workarouds for possible bugs is a good enough reason to keep the graphqlTypeName https://github.com/Husqvik/GraphQlClientGenerator/pull/130