Does this C# code require correction which is although working fine?

[Monica Shiralkar]

I have the below C# code which is in a Web API being called from Java Script file :

public IActionResult IncrementQuantity([FromBody] dynamic userData)
        {
            var parsedUserData = JsonConvert.DeserializeObject(userData.ToString());

            int pie = _pieRepository.GetPieById(Int32.Parse(parsedUserData.pieId.ToString()));
           

Does the below lines require correction  ?

public IActionResult IncrementQuantity([FromBody] dynamic userData)

I used dynamic here since it I am not knowing the type of userData coming from the JS (the JS file below in the POST)

var parsedUserData = JsonConvert.DeserializeObject(userData.ToString());

I did userData.ToString() as it requires it be be in String to be able to parse it.

int pie = _pieRepository.GetPieById(Int32.Parse(parsedUserData.pieId.ToString()));

I did parsedUserData.pieId.ToString() since _pieRepository.GetPieById requires int and for converting to int first I have to convert to string.

Below is the JS code calling the above Web API controller:

var pieId = $(this).attr("data-pieId");
var userData = { 'pieId': pieId }
            {
              $.ajax({

                    type: "POST",
                    url: "/api/ShoppingCart/IncrementQuantity",
                    data: JSON.stringify(userData),
                    contentType: "application/json",
                    success: function (response) {

Thanks

[Stephan van Hulst]

Yes. Don't manually deserialize the request body. Rely on the built-in deserialization mechanism:

public IActionResult IncrementQuantity([FromBody] IncrementQuantityRequest request)
{
  var id = request.PieId;

  ...
}

public sealed class IncrementQuantityRequest
{
  [BindProperty(Name = "pieId")]
  public string PieId { get; private set; }
}

However, for such a simple request (you're essentially just sending the ID of the resource you want to increment), it's better to just use a primitive type instead of an object, and when the parameter represents an ID, it's usually better to make it into a route parameter:

[HttpPut("items/{productId}/amount", Name = "Items.Amount.Set")]
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public IActionResult SetItemAmount([FromRoute(Name = "productId")] string productId, [FromBody] int amount)
{
    if (amount < 0)
    {
        return BadRequest("The amount may not be negative.");
    }

    if (!ShoppingCart.SetItemAmount(productId, amount))
    {
        return NotFound($"The product with ID '{productId}' could not be found.");
    }

    return Ok();
}

Note that I used a PUT request instead of a POST request here, because sending the same request twice will have no effect. I also used a string to represent the product ID. Never use integers for IDs, because you are not going to perform arithmetic on IDs.

[Monica Shiralkar]

Thanks. These are useful learnings.

As you have rightly said I should use primitive type in the below case and not object . Understood that but have a doubt not directly related to it.

Suppose the request would have been a bit complex  ,then for that object would usage of dynamic have been okay ? If not so,then would there have been a way to know the type and use it instead of dynamic .

public IActionResult IncrementQuantity([FromBody] dynamic userData)

[Stephan van Hulst]

No, dynamic is not okay. I don't think there are many modern applications that warrant the use of dynamic.

The writer of the web API decides the data types used in the requests and responses. YOU are the writer of the web API, and you are writing the API in C#. That means that you have complete control over the data types used. Use strong C# types.

It's the client who has to adapt to the web API.

[Monica Shiralkar]

One way to avoid dynamic in this line of code is by replacing it with object.

[Stephan van Hulst]

And how will that help you exactly?

[Monica Shiralkar]

The value comes from below JavaScript code :

var userData = { 'pieId': pieId }
 

To the below web api code :

public IActionResult IncrementQuantity([FromBody] dynamic userData)

Based on the Java Script code , I thought it is a JSON string and therefore first tried making the type of userData in the web api method parameter as string. However that gave me an error saying cannot convert to string.
Next , I tried with var but compiler did not allow that since parameter cannot have type as var.

After this I am left with only object and dynamic.

[Stephan van Hulst]

Why did you think it would be a string? The JSON you posted isn't a string. It's an object containing a property named 'pieId'. That means that you need to create a matching class in C# that resembles an object containing a property named 'pieId'. I showed you exactly how to do that in the post where I tell you to depend on the built-in deserialization mechanism of ASP.NET Core.

Using 'object' won't help you because then you can't access any of its members. Using 'dynamic' will work, but it's bad practice.

[Monica Shiralkar]

Thanks. I will do that way and remove the dynamic.

I have a question.As you said, using dynamic will be wrong  for this case then what can be a case where using dynamic would not be wrong ?

[Stephan van Hulst]

There are almost no cases where dynamic is appropriate.

[Monica Shiralkar]

Okay.So why does such a feature exist when it should never be used.

[Stephan van Hulst]

Because people love features that makes their life easier in the short term.

[Monica Shiralkar]

Thanks