Naming of ApiController

[csantero]

I know this is a bit of a bike-shedding discussion, but it bothers me that this is called the same thing as WebAPI's System.Web.Http.ApiController. Seems like it raises quite a bit potential for confusion. Could we maybe rename it to JSONAPI.Http.JsonApiController?

[SphtKr]

Honestly I'm leaning pretty strongly against this one. Couple of reasons:

• Simplest one, that's what namespaces are for...if confusion is a concern, we can make sure to use the namespace-qualified version in example code, but really that's the whole point of putting things in a namespace: so that you can have multiple classes with the same name.
• The above argument breaks down quickly when you might use two same-named classes from different namespaces in the same code context--I've particularly regretted naming a model class UserPrincipal when using the Microsoft-provided UserPrincipal at the same time. But, I can't really see a situation where this would ever become an issue in the case of ApiController.
• I think the fact that our ApiController classes have a generic parameter is a great big flag that this is something different, which helps avoid confusion.
• Really we have two ApiControllers, one in JSONAPI and one in JSONAPI.EntityFramework. We may very well have more later (far out on my mental roadmap is adding NHibernate support via another assembly parallel to JSONAPI.EntityFramework), which would imply that we needed different names for all of them, which would begin to get ugly.
• Generally, I would like JSONAPI[.*].Http.ApiController to be a drop-in replacement for the upstream ApiController as much as possible. The reasons include developer comfort, but also that it should be possible to also serve XML output or perhaps even default Json.NET-format JSON or some other MediaType out of our ApiControllers as well. This may not be currently practical, but I've hoped it could be made so--that's one of the design goals of the upstream ApiController, to be able to emit multiple media types and be largely serializer-agnostic. Naming the class more specifically would I think imply that that's not the idea. Naming it the same I think implies that we're trying to honor the upstream contract as well, albeit with some limitations or restrictions.

Not to blast this, but it's worth documenting some of this as they're design goals I probably haven't expressed elsewhere.

[SphtKr]

Was looking for some best practice discussion on this, and found this question...both of the existing answers were correct so I felt compelled to add my own :-)

[csantero]

Simplest one, that's what namespaces are for...if confusion is a concern, we can make sure to use the namespace-qualified version in example code, but really that's the whole point of putting things in a namespace: so that you can have multiple classes with the same name.

Computers are happy to make this distinction, but the human brain isn't, especially if it belongs to a newcomer to the library (or even to a newcomer to the Web API ecosystem in general). It's more likely to latch onto the name of the class itself.

The above argument breaks down quickly when you might use two same-named classes from different namespaces in the same code context--I've particularly regretted naming a model class UserPrincipal when using the Microsoft-provided UserPrincipal at the same time. But, I can't really see a situation where this would ever become an issue in the case of ApiController.

Sure, the user is unlikely to use System.Web.Http.ApiController and our ApiController<> at the same time. But they are very likely to have a using System.Web.Http; in the same file. This makes the likelihood of using the wrong one high, if they don't know any better.

Really we have two ApiControllers, one in JSONAPI and one in JSONAPI.EntityFramework.

I didn't realize we did this until yesterday and it strikes me as something we should try to consolidate. Shouldn't everything the EF ApiController does be performed by the EntityFrameworkMaterializer, or some other internal dependency? The point of the controller is to expose a certain set of actions for Web API, but both of our ApiControllers expose this same set of methods. The heavy lifting that the EF controller does should be refactored into some internal class, to simplify the public API surface.

Ideally, the user would extend our (only) ApiController, and all they have to do is provide an implementation of IMaterializer (or whatever this other dependency is called that would do what the EF ApiController does now). If they need to do custom per-endpoint processing they can still override the methods, but now they no longer have to pick an ApiController implementation.

Generally, I would like JSONAPI[.*].Http.ApiController to be a drop-in replacement for the upstream ApiController as much as possible. The reasons include developer comfort, but also that it should be possible to also serve XML output or perhaps even default Json.NET-format JSON or some other MediaType out of our ApiControllers as well. This may not be currently practical, but I've hoped it could be made so--that's one of the design goals of the upstream ApiController, to be able to emit multiple media types and be largely serializer-agnostic. Naming the class more specifically would I think imply that that's not the idea. Naming it the same I think implies that we're trying to honor the upstream contract as well, albeit with some limitations or restrictions.

The only reason someone is going to be using these classes is because they are using json-api. The nature of the spec means that a MediaTypeFormatter alone is not powerful enough to do everything the spec needs, which is why we have a custom ApiController to begin with. What does our ApiController<> give you that you would want if you were outputting XML? Especially regarding our other conversation regarding the MetadataManager, it seems that our controller is only going to be more intertwined with json-api-specific concerns. Calling it JsonApiController just seems to clarify its role. (JsonApiApiController would be even clearer, but that just sounds dumb)

---

If you still feel strongly about this, I'm willing to drop it, we've spent enough time on this topic already. I am interested in your thoughts about consolidating our own ApiControllers, though.

[SphtKr]

No, this is a good discussion, though I'm still leaning towards leaving it as is--the SO question I linked to helped firm up in my mind why, and it's like the Scala Map example the 2nd answer gives: the hope is that our ApiControllers will match the interface of the upstream one very closely and be a superset, therefore naming it the same makes sense to a programmer. Also the "Smurf Naming" discussion referenced by the original poster resonates with me...BTW arguably if we change it we should name it the even worse JsonApiApiController :-/

The only reason someone is going to be using these classes is because they are using json-api. The nature of the spec means that a MediaTypeFormatter alone is not powerful enough to do everything the spec needs, which is why we have a custom ApiController to begin with. What does our ApiController<> give you that you would want if you were outputting XML?

You're right, it's not at all that our library gives you much of anything that you'd want if you weren't needing to provide a JSON API interface--there's not much reason to use our library if you're only providing another format. But, if you have an Ember web app, and you want to also provide XML API support (say, for a Windows Phone app), should we make you build a whole parallel set of vanilla ApiControllers at a different API URL path? I hope not... It may turn out at the end that they have to, but again that's the design of the upstream ApiController (or arguably of the WebAPI pipeline): to be able to support multiple output formats simultaneously.

I didn't realize we did this until yesterday and it strikes me as something we should try to consolidate. Shouldn't everything the EF ApiController does be performed by the EntityFrameworkMaterializer, or some other internal dependency?

Now this is an interesting point. I think I broke these out separately before I designed IMaterializer. The question of whether the functionality currently in the Put, Post, and Delete methods of the EF ApiController could be broken up and rolled into IMaterializer interface methods is a good one, and yes it might let us consolidate down to a single ApiController implementation. This probably needs to be looked at anyway, for a few reasons.

First, I've found that I usually have to override the non-Get verb methods in my ApiController subclasses anyway to add my business logic, and ATM reusing the superclass implementations isn't easy. The main reason I end up having to do this is to add authorization logic, so I've opened #35 to look at adding this function to IMaterializer, which is right in line with what you're saying.

Second, the fact that the spec allows you to post either a single resource object or an array of resource objects interchangeably threw me for a loop--literally, I had to add the foreach loop in all three verb implementations (and there's a weird hack in JsonApiFormatter.ReadFromStream(...) to deserialize a single object as a List!) because WebAPI doesn't let you overload the verb methods based on the incoming type! This is what makes reusing the superclass method difficult, because you'd have to loop over the results twice in most cases, which is undesirable. I've thought about adding signatures like [NonAction] public override IList<T> Post(T postedObj) { } (note, a single T as the incoming object) and then calling that within the loop in the actual action method, so folks could pretend that they could overload that way, but that has its downsides too. Anyway, finding a way to make that logic more reusable is definitely desirable.

So back to the naming question: let's look at the consolidation thing you brought up first. Once we've resolved that (and likely had other things mature over time) it may inform the naming question better. I've opened #36 to track this. I'm going to create a deferred label and tag this for later. We might want to push that to a 0.4.0 milestone though, we've got a lot of changes queued that I think might be worth pushing as 0.3.0 soon(ish).

[csantero]

Sounds good