NamedTuple with default arguments

[vladfaust]

Currently NamedTuple doesn't allow to have default arguments, i.e. this code won't compile:

def foo(x : NamedTuple(a: Int32, b: Int32? = nil))
  pp x
end

foo(x: {a: 42})

It would be great if it worked.

Use case

I'm currently implementing an API client for Stripe. Let's see this method: https://stripe.com/docs/api/tokens/create_card. card argument is an object. Currently I can't do this:

class Stripe::Client
  def create_card_token(
    card : NamedTuple(
      exp_month: Int32,
      exp_year: Int32,
      number: String,
      currency: String? = nil,
      cvc: Int32? = nil,
      name: String? = nil,
      address_line1: String? = nil,
      address_line2: String? = nil,
      address_city: String? = nil,
      address_state: String? = nil,
      address_zip: String? = nil,
      address_country: String? = nil)? = nil,
    customer : String? = nil
  )
  end
end

client.create_card_token(card: {exp_month: 12, exp_year: 2019, number: "4242424242424242", cvc: 123})

But instead I have to do this:

class Stripe::Client
  record CreateCardToken::Card,
    exp_month : Int32,
    exp_year : Int32,
    number : String,
    currency : String? = nil,
    cvc : Int32? = nil,
    name : String? = nil,
    address_line1 : String? = nil,
    address_line2 : String? = nil,
    address_city : String? = nil,
    address_state : String? = nil,
    address_zip : String? = nil,
    address_country : String? = nil

  def create_card_token(
    card : CreateCardToken::Card? = nil,
    customer : String? = nil
  )
  end
end

client.create_card_token(card: Stripe::Client::CreateCardToken::Card.new(
  number: "4242424242424242",
  exp_month: 12,
  exp_year: 2019,
  cvc: 123,
))

Note: Stripe has a not-so-OOP API, which doesn't allow to use a single Stripe::Card object for CRUD methods, so I have to define arguments manually for each method.

[straight-shoota]

So, what's the downside of having individual records (for each purpose, if that's required) vs. named argument? I fail to see why that's an issue.

[vladfaust]

@straight-shoota the need to explicitly specify the record (i.e. Stripe::Client::CreateCardToken::Card) for each argument when calling a method. The use-case is more complicated, for example https://stripe.com/docs/api/customers/create has two complex arguments with one having a nested object, which leads to a such code:

class Stripe::Client
  # To avoid module/struct errors
  struct CreateCustomer::Shipping
  end

  record CreateCustomer::Shipping::Address,
    line1 : String,
    city : String? = nil,
    country : String? = nil,
    line2 : String? = nil,
    postal_code : String? = nil,
    state : String? = nil
  end

  record CreateCustomer::Shipping,
    address : CreateCustomer::Shipping::Address,
    name : String,
    phone : String? = nil
  end

  record CreateCustomer::TaxInfo,
    tax_id : String,
    type : Type do
    enum Type
      Vat
    end
  end

  def create_customer(
    account_balance : Int32? = nil,
    coupon : String? = nil,
    default_source : String? = nil,
    description : String? = nil,
    email : String? = nil,
    invoice_prefix : String? = nil,
    metadata : Hash? = nil,
    shipping : CreateCustomer::Shipping? = nil,
    source : String? = nil,
    tax_info : CreateCustomer::TaxInfo? = nil
  )
end

client.create_customer(
  account_balance: 10,
  shipping: Stripe::Client::CreateCustomer::Shipping.new(
    address: Stripe::Client::CreateCustomer::Shipping::Address.new(
      line1: "foo",
      line2: "bar"
    ),
    name: "John"
  ),
  tax_info: Stripe::Client::CreateCustomer::TaxInfo.new(
    tax_id: "123",
    type: :vat
  )
)

Which, in the case of solved issue, could be reduced to:

class Stripe::Client
  enum TaxType
    Vat
  end

  def create_customer(
    account_balance : Int32? = nil,
    coupon : String? = nil,
    default_source : String? = nil,
    description : String? = nil,
    email : String? = nil,
    invoice_prefix : String? = nil,
    metadata : Hash? = nil,
    shipping : NamedTuple(
      address: NamedTuple(
        line1: String,
        city: String? = nil,
        country: String? = nil,
        line2: String? = nil,
        postal_code: String? = nil,
        state: String? = nil),
      name: String,
      phone: String? = nil,
    )? = nil,
    source : String? = nil,
    tax_info : NamedTuple(
      tax_id: String,
      type: TaxType,
    )? = nil
  )
  end
end

client.create_customer(
  account_balance: 10,
  shipping: {
    address: {
      line1: "foo",
      line2: "bar",
    },
    name: "John",
  },
  tax_info: {
    tax_id: "123",
    type:   :vat,
  }
)

[vladfaust]

We still have a little time until 0.27. Is it hard to implement?

[j8r]

Your example with the record sound OK for the job (56 lines vs 47 lines). Why adding complexity and another feature to NamedTuple? Their future is even uncertain, structs are more powerful for the same footprint.

[vladfaust]

1. There is a "job" to send a single request. Adding additional objects which will not be used anywhere else is an overhead.
2. You're comparing code lines including internal API. The implementation within Stripe scope isn't that important compared to a public API. So please compare these instead and see the difference:

client.create_customer(
  account_balance: 10,
  # Imagine a developer using this API needing to know about all these Stripe::Client::CreateCustomer objects,
  # which may be many for every API method, and there are at least 50 methods in Stripe API
  shipping: Stripe::Client::CreateCustomer::Shipping.new(
    address: Stripe::Client::CreateCustomer::Shipping::Address.new(
      line1: "foo",
      line2: "bar"
    ),
    name: "John"
  ),
  tax_info: Stripe::Client::CreateCustomer::TaxInfo.new(
    tax_id: "123",
    type: :vat
  )
)

client.create_customer(
  account_balance: 10,
  shipping: {
    address: {
      line1: "foo",
      line2: "bar",
    },
    name: "John",
  },
  tax_info: {
    tax_id: "123",
    type:   :vat,
  }
)

3. adding complexity and another feature to NamedTuple

Default arguments is a natural thing and present in many other parts of the language. I feel this feature missing in NamedTuple.

4. Their future is even uncertain

Proofs?

5. structs are more powerful for the same footprint

Elaborate please.

[j8r]

NamedTuple is a struct, thus are represented the same than other structs in the stack memory. However with structs you can have methods, ivars and use composition/inheritance that aren't possible in a NamedTuple. Often you can use structs instead of named tuples.

[vladfaust]

In this case I'd prefer to use NamedTuple for the sake of simplicity, readability and lesser code.

[asterite]

Create a record with default arguments. In general you should never use named tuples and I regret adding them to the language.

[vladfaust]

I don't read the issue, but if I see %feature_name I say it should be removed from the language

Whom do I write these use-cases for and defend my points trying to elaborate as much as possible? Please spend some time diving into someone's pain. If it wasn't, I wouldn't create the issue.

[asterite]

Not everything needs to be a one liner.

[j8r]

@vladfaust I understand, but adding complexity adds also unnecessary pain that can be avoided to the core devs and contributors - more code to maintain, to learn, to test, to document, to debug in case of issues...

[vladfaust]

@asterite,

Not everything needs to be a one liner.

Your words don't make sense. What conclusion should I make from it? "You issue doesn't matter"? Alright, I understand that. And if it was some kind of personal project, I could understand your not-giving-a-f attitude towards it, but Stripe is a world-class business. It's the bridge between Crystal being a hobby language and startup language.

Imagine a Stripe.cr user needing to go to its Crystal API docs to look up for a struct name to use for this particular method. Imagine them writing this:

client.create_customer(
  account_balance: 10,
  shipping: Stripe::Client::CreateCustomer::Shipping.new(
    address: Stripe::Client::CreateCustomer::Shipping::Address.new(
      line1: "foo",
      line2: "bar"
    ),
    name: "John"
  ),
  tax_info: Stripe::Client::CreateCustomer::TaxInfo.new(
    tax_id: "123",
    type: :vat
  )
)

Instead of this:

client.create_customer(
  account_balance: 10,
  shipping: {
    address: {
      line1: "foo",
      line2: "bar",
    },
    name: "John",
  },
  tax_info: {
    tax_id: "123",
    type:   :vat,
  }
)

The code in the first example is repeating itself:

shipping: Stripe::Client::CreateCustomer::Shipping.new( address: Stripe::Client::CreateCustomer::Shipping::Address

And, as I said before, there are 50+ methods in Stripe API. That means hundreds of long struct names to remember.

upd: again, I'm too emotional. What's wrong with me?! Sorry, @asterite

[j8r]

I'm sure you can have a good looking API using a struct and setting the variables with properties to replace the default values. Something like https://stripe.com/docs/api/customers/create?lang=ruby and https://stripe.com/docs/api/customers/create?lang=ruby

[vladfaust]

No offence, @j8r, but really, you don't seem like to understand the issue. It is about nested arguments with omitable arguments preserving strong typing and simplicity.

I'm tired of explaining the thing this many times. If you think that the issue is bullshit and I'm an arrogant prick crying big from nothing, close the issue. (upd: yes, I am 😕)

Otherwise, please, label it and put to the dark corner, because we, Crystallers, don't really need simple API bindings, we want to be as much verbose as possible and still call ourselves Ruby-like, we don't care about developer's level of live and of course we don't want to see Crystal in production in real-world start-ups.

upd: stop crying, Vlad

[asterite]

Please detail how to implement this, and, if possible, attach a PR. It's easy to propose things but then you realize it doesn't mix well with all of the other features of the language and it's super hard to implement.

In general, anything that involves changing the language has "no" as an answer, and only after thoroughly analyzing all the implications can be put into discussion.

[vladfaust]

https://github.com/crystal-lang/crystal/issues/7001#issuecomment-433849314

I did ask if it's hard to implement. Why should I spend so much time and nerves just to see the answer to it? It's a rhetorical question.

[asterite]

The problem is it takes time to figure out if it's hard to implement. If you do it yourself it will save us a lot of time.

[asterite]

Also at this point there are many features in the language that are buggy. I think it's better first to fix those bugs and strive for a stable and consistent language, then keep adding features that are buggy (autocasting is buggy and I can't see why a quick and dirty implementation of this won't be too)

[vladfaust]

Okay, as I'm zero in the compiler itself, all I'm left to do is continue working with what we have, hoping to earn enough to sponsor the language well. I'd be thankful for proper issue labeling.

[ysbaddaden]

Looking at your needs, and the Stripe Ruby & Python SDKs, I see no reason to not use a Hash and/or kwargs (**args). NamedTuple is meant to take and pass kwargs around, it's not meant to replace Hash.

[vladfaust]

What I want, @ysbaddaden, is strictly typed args to raise errors on compilation time. That what differs Crystal from Stripe Ruby & Python SDKs ¯\_(ツ)_/¯. **args : **T) forall T could potentially solve the issue, but it would require a ton of macros.

[RX14]

https://carc.in/#/r/5dps

you can easily write this DSL, typed well, just requires some small boilerplate for nested records.

Vote close.

[RX14]

Even produces nice errors: https://carc.in/#/r/5dpx

[vladfaust]

Hey @RX14 we've discussed that on gitter and came to understanding that in this case I don't need records because these arguments aren't going to be reused. Is there a way to do the same but avoiding records? I've tried it with merge but got no luck on raising on missing arguments in compile time...

[RX14]

@vladfaust use .as to assert the "shape" of the named tuple.

[vladfaust]

Alright, I've written a solution using macros. Like it so far, but may have some unhandled edge cases:

private macro validate(arg, source, &block)
  {% for exp in block.body.expressions %}
    {% if !exp.block %}
      {%
        declaration = exp.args[0]

        if declaration.value.is_a?(Nop)
          raise "#{arg}[#{declaration.var}] (#{declaration.type}) is a required argument" unless source.named_args[declaration.var]
        end

        unless source.is_a?(NilLiteral)
          actual_type = source.named_args[declaration.var]
          match = false

          if declaration.type.is_a?(Union)
            match = declaration.type.types.any? do |t|
              (actual_type.is_a?(NilLiteral) && t.resolve.nilable?) || (!actual_type.is_a?(NilLiteral) && actual_type.resolve <= t.resolve)
            end
          else
            match = (actual_type.is_a?(NilLiteral) && declaration.type.resolve.nilable?) || (!actual_type.is_a?(NilLiteral) && actual_type.resolve <= declaration.type.resolve)
          end

          raise "#{arg}[#{declaration.var}] must be #{declaration.type} (given #{actual_type})" unless match
        end
      %}
    {% else %}
      {% name = exp.args[0] %}
      {% new_source = source.named_args[name.symbolize] %}
      {% if new_source.is_a?(Generic) && new_source.name.stringify == "NamedTuple" %}
        validate({{"#{arg}[#{name}]".id}}, {{new_source}}) {{exp.block}}
      {% else %}
        {% raise "Expected #{arg}[#{name}] to be a NamedTuple, not #{new_source}" %}
      {% end %}
    {% end %}
  {% end %}
end

Usage is pretty straightforward:

def create_card_token(
  card : T? = nil
) forall T
  validate card, {{T}} do
    type exp_month : Int32
    type exp_year : Int32
    type number : String
    type currency : String? = nil
    type cvc : Int32? = nil
    type name : String? = nil
    type address_line1 : String? = nil
    type address_line2 : String? = nil
    type address_city : String? = nil
    type address_state : String? = nil
    type address_zip : String? = nil
    type address_country : String? = nil
  end
end

create_customer(
  account_balance : Int32? = nil,
  coupon : String? = nil,
  default_source : String? = nil,
  description : String? = nil,
  email : String? = nil,
  invoice_prefix : String? = nil,
  metadata : Hash? = nil,
  shipping : T? = nil,
  source : String? = nil,
  tax_info : U? = nil
) forall T, U
  validate shipping, {{T}} do
    type address do
      type line1 : String
      type city : String? = nil
      type country : String? = nil
      type line2 : String? = nil
      type postal_code : String? = nil
      type state : String? = nil
    end

    type name : String
    type phone : String? = nil
  end

  validate tax_info, {{U}} do
    type tax_id : String
    type type : Customer::TaxInfo::Type # Custom types
  end
end

Getting nice errors as well:

Error in example.cr:12: Expected shipping[address] to be a NamedTuple, not Int32

customer = stripe.create_customer(source: token, shipping: {
                  ^~~~~~~~~~~~~~~

Error in example.cr:12: instantiating 'Stripe#create_customer()'

customer = stripe.create_customer(source: token, shipping: {
                  ^~~~~~~~~~~~~~~

in src/stripe/methods/core/customers/create_customer.cr:14: shipping[address][line2] must be String | ::Nil (given Int32)

    validate shipping, {{T}} do

Error in example.cr:12: tax_info[type] (Customer::TaxInfo::Type) is a required argument

customer = create_customer(source: token, shipping: {

Macros are power :muscle:

[vladfaust]

I have another use-case: https://github.com/vladfaust/params.cr/issues/11