So we had an argument whether this question is on topic: RESTful movie shop
I see this question as off-topic although it seems to satisfy most rules.
There's code. It works. It's real...
But the questions asked by OP does not seem to look for a code review.
He asks if its ok to use verbs in RESTful API and similar things. If you replace all the code snippets with descriptions of HTTP requests, query parameters, response statuses, etc. all the text of the post is still valid, because it never actually talks about the code. It just uses the code to show us how the HTTP API is designed and asks if the design follows best practices.
To me it seems that OP created a design, implemented it, then stopped and thought maybe he should have the design reviewed first and he used the code to describe the design to potential reviewers because that's what he already had.
A reference to rule that says it may be off-topic is here in the help/dont-ask:
Higher-level architecture and design of software systems
We review code, not concepts, diagrams, or outlines. Whiteboard-style questions may be suitable on Software Engineering if they are specific.
But it's not clear whether the presence of code makes it on-topic or not.
So how is it?
I am including copy of the post here too to keep the exact wording at the time and have it self-contained:
Just would like to ask if these custom web API naming follow best practice? Below are some of the web API methods I have created for my pet project using ASP.NET .NET Framework 4.7.
I understand that the standard CRUD naming convention especially when I use the scaffolding option. For example, this Cart controller web API, the scaffolded code are.
// GET: api/Carts
public IQueryable<Cart> GetCarts()
{
return db.Carts;
}
// GET: api/Carts/5
[ResponseType(typeof(Cart))]
public async Task<IHttpActionResult> GetCart(int id)
{
Cart cart = await db.Carts.FindAsync(id);
if (cart == null)
{
return NotFound();
}
return Ok(cart);
}
// PUT: api/Carts/5
[ResponseType(typeof(void))]
public async Task<IHttpActionResult> PutCart(int id, Cart cart)
{
if (!ModelState.IsValid)
{
return BadRequest(ModelState);
}
if (id != cart.Id)
{
return BadRequest();
}
db.Entry(cart).State = EntityState.Modified;
try
{
await db.SaveChangesAsync();
}
catch (DbUpdateConcurrencyException)
{
if (!CartExists(id))
{
return NotFound();
}
else
{
throw;
}
}
return StatusCode(HttpStatusCode.NoContent);
}
// POST: api/Carts
[ResponseType(typeof(Cart))]
public async Task<IHttpActionResult> PostCart(Cart cart)
{
if (!ModelState.IsValid)
{
return BadRequest(ModelState);
}
db.Carts.Add(cart);
await db.SaveChangesAsync();
return CreatedAtRoute("DefaultApi", new { id = cart.Id }, cart);
}
// DELETE: api/Carts/5
[ResponseType(typeof(Cart))]
public async Task<IHttpActionResult> DeleteCart(int id)
{
Cart cart = await db.Carts.FindAsync(id);
if (cart == null)
{
return NotFound();
}
db.Carts.Remove(cart);
await db.SaveChangesAsync();
return Ok(cart);
}
But, I don't use much of those CRUD method above. So, I added my own like below:
[Route("api/Carts/GetClearAllCart/{id:int}")]
public async Task<IHttpActionResult> GetClearAllCart(int id)
{
var carts = db.Carts.Where(c => c.UsersId == id);
if (carts == null)
{
return NotFound();
}
db.Carts.RemoveRange(carts);
await db.SaveChangesAsync();
return Ok(carts);
}
[Route("api/Carts/GetClearOne/{id:int}")]
public async Task<IHttpActionResult> GetClearOneItemFromCart(int id)
{
var cart = db.Carts.SingleOrDefault(c => c.SeatId == id);
if (cart == null)
{
return NotFound();
}
db.Carts.Remove(cart);
await db.SaveChangesAsync();
return Ok();
}
[Route("api/Carts/GetClearAllByUserId/{id:int}")]
public async Task<IHttpActionResult> GetClearAllByUserId(int id)
{
var cart = db.Carts.Where(c => c.UsersId == id);
if (cart == null)
{
return NotFound();
}
db.Carts.RemoveRange(cart);
await db.SaveChangesAsync();
return Ok();
}
[Route("api/Carts/GetCartsByUserId/{id:int}")]
public IQueryable<CartVM> GetCartsByUserId(int id)
{
var cartVMList = new List<CartVM>();
var selectedCart = db.Carts.Where(c => c.UsersId == id);
foreach (var item in selectedCart)
{
var cartVM = new CartVM();
cartVM.MovieTitle = item.MovieTitle;
cartVM.HallNo = item.HallNo;
cartVM.MovieDateTime = item.MovieDateTime;
cartVM.Seats = item.Seats;
cartVM.SeatId = item.SeatId;
cartVMList.Add(cartVM);
}
return cartVMList.AsQueryable();
}
// GET: api/Movies
public IQueryable<MovieVM> GetMovies()
{
var movieVMList = new List<MovieVM>();
foreach (var item in db.Movies)
{
var movieVM = new MovieVM();
movieVM.MovieTitle = item.MovieTitle;
movieVM.Id = item.Id;
movieVM.PhotoFile = item.PhotoFile;
movieVMList.Add(movieVM);
}
return movieVMList.AsQueryable();
}
// GET: api/Movies/5
[ResponseType(typeof(Movie))]
public async Task<IHttpActionResult> GetMovie(int id)
{
Movie movie = await db.Movies.FindAsync(id);
if (movie == null)
{
return NotFound();
}
return Ok(movie);
}
I read that we should focus on the noun for RESTful API instead of verbs which is SOAP. How can we create a custom web API only with noun and without verbs? Any comment on my custom web API methods?
And comments:
Questions about best practices are outside the scope of this site. – slepic
This looks on-topic to me IDK why people are saying it's not (They don't have 3k yet so there's no close votes either). Your question (description) seems somewhat high level, but your code looks fairly solid. – Peilonrayz
@Peilonrayz yes, the code looks good for a code review, but the OP doesn't seem to be interested as much in code review as he is interested in a design review and best practices with RESTful APIs. That makes it off topic. A good code review would probably provide answers to OP's questions as well, but it shouldn't be the main focus. As it Is now, it seems to fit better on Software Engineering Stack Exchange. Unfortunately, there is no such migration path. – slepic
@slepic Please can you provide links to Meta that show that this is indeed off-topic. – Peilonrayz
@Peilonrayz section "Higher-level architecture and design of software systems" of codereview.stackexchange.com/help/dont-ask – slepic
@slepic That's if there's no code. And there is clearly code. – Peilonrayz
I understand it Is subject to interpretation. I lean more towards off-topic, but I understand it may be viewed differently. Nevertheless, I believe SE has a better potential to satisfy the OP And He should go ask there for his own sake regardless of this question being closed or not. – slepic
If I post a code snippet and then talk about my holiday, am I asking for code review? – slepic
@slepic It doesn't matter if SE exists or not. The only thing that makes a question off-topic on Code Review is Code Review's rules. Additionally your simile has no likeness to the current situation and only unjustly paints the question in a bad light. Please take this to meta if you have a problem. – Peilonrayz
