More descriptive names - Githubissues

paldepind commented 5 years ago

As a newcomer to Affjax, I found the names of the various types and functions to be a bit confusing. As an example of what I found confusing, there is a type named Response and a typed named AffjaxResponse. They sound like the same thing. Furthermore, Response isn't actually a response. It's a description of how to interpret the body of a response.

In this PR I've tried to rename things in ways that, at least to me, makes more sense. Obviously naming is somewhat of an opinion. But, below is a list of the things that the PR renames with my rationale for the renaming.

Response to ResponseFormat. This type isn't actually a response. It's a description about which format the body of a response should be interpreted as.
AffjaxResponse to Response. This is the actual type of a response.
Request to RequestBody. This type is not a request but the type of things that can be used as the body of a request.
AffjaxRequest to RequestOptions. This type represents a record of options specifying how a request should be made.
affjax to request. The name "affjax" doesn't really communicate anything. The name request is appropriate since this is the function used to make requests.
Affjax removed. Affjax a is equivalent to Aff (Response a). IMO the Affjax type only obfuscated the types and made it harder to see that the functions in the library return responses in the Aff monad.

As an example this is the type of post today:

post :: forall a. Response a -> URL -> Request -> Affjax a

And this is the type of post in this PR:

post :: forall a. ResponseFormat a -> URL -> RequestBody -> Aff (Response a)

I'd argue the latter is easier to understand. I find it confusing that the first argument has the type Response. Why do I need to give a response to a function that is supposed to make a request and return a response? Also, the name RequestBody makes it explicit which argument is the actual payload for the request.

kritzcreek commented 5 years ago

I like all of these changes 👍 Of course this will cause a breaking release, any plans for that soonish @garyb ?

garyb commented 5 years ago

I like them too.

I had no particular plans for doing anything on this at the moment, but now this has been opened there's a few things I can think of trying out, so will see how they go :wink:. Otherwise I'm fine with making a new breaking release just for this anyway.

paldepind commented 5 years ago

I'm happy that you like the changes and thanks for merging :+1:

purescript-contrib / purescript-affjax

More descriptive names #126