search

mattsta / stripe-erlang

Erlang interface to the stripe.com API
27 stars 23 forks source link

Minor code cleanup and contribution aid #8

Closed jaredmorrow closed 9 years ago

jaredmorrow commented 9 years ago

The existing indention style of 2 spaces is not the common Erlang/Emacs style shipped with Erlang, so I added headers to enforce the 2-space style to make it easier for people to contribute without messing up your indention.

The second change was to remove the unneeded binary_to_atom calls since we can just as easily pattern match on binaries themselves. Tests were reran to confirm they still pass.

mattsta commented 9 years ago

Thanks for these too. The binary_to_atom issue is because I really hate typing <<"thing">> when I could just type thing, but you're right, it's cleaner to use the binaries directly. :whale:

jaredmorrow commented 9 years ago

@mattsta thanks for merging these. I have one other branch for paginated support (listing customers, charges, etc.), but it was a much bigger change. Is this something interesting to you?

mattsta commented 9 years ago

Pagination would be nice to add too. I've only ever processed 3 production Stripe payments myself, so the need for pagination hasn't come up before. :)

Any feedback about pain points with using the stripe-erlang API would be great too.

Some things I'd like to change eventually include: stop putting the API key in an env variable (but adding a Config parameter on every function seems messy too) and/or requiring record use for types.

Records probably need to be refactored in favor of maps eventually. It'll make upgrading a lot easier when all modules using this API don't have to be recompiled because records in headers changed.

But, the tradeoff is the map interface seems uglier/more verbose.

Records:

Value = Result#type.field.

Maps (fields may even end up as binaries too):

#{ field := Value } = Result.
% or
Value = maps:get(field, Result).
jaredmorrow commented 9 years ago

As much as I really don't like Erlang record syntax compared to almost any programming language, they do provide a really easy mechanism for doing list comprehensions over data and having that data type checked against a record. I haven't used maps yet, but it seems in this case records are a decent solution.

I haven't had a lot of pain points with the library to be honest. I was confused by a couple of things, but those are the things I put in these PRs. I've been cranking on my tool that uses this library, but when I get some free time I'll clean up the pagination branch and get something submitted.

BTW, I work with Tom Santero who had nothing but good things to say about you. Small world.