C#•2y ago

✅ Pass status as parameter in controller

Hello, I am trying to retrieve a list of Active Employees, for this purpose, I have a separate domain where I keep my logic by having a GetActiveEmployees method. Then in my controller, I am calling the method but I want my endpoint to be : GET api/employees?status=active. My question is how can I pass the status which should always be active. At the moment the issue is that whatever I put in as status, it still works, whereas I want to have it work only if the status is active. Here is my domain logic : https://gyazo.com/7eb168cb68a632d14a28b3c1f1ecc3c9 and here is my controller : https://gyazo.com/8c907a7ddcadd79b6a95f4ef0a1dc185

Gyazo

36 Replies

Pobiega•2y ago

well, your domain logic checks that IsActive is true. There are no string comparisons involved at all

dragzunOP•2y ago

so I should also compare the string if it is active or not ? and if so, what should I return if it is not active

Pobiega•2y ago

No you need to think about what you are trying to do GetActiveEmployees currently doesnt care about its input, it returns the same thing regardless of what you give it what do you want to happen if I type "Cheese" as my status? "Inactive"? "ACtIve"?

dragzunOP•2y ago

To not return the employees, basically I might also get employees which are not active in the future, but for now, i just want the status to only work as active for these types of employees

Pobiega•2y ago

so "cheese" should just return nothing?

dragzunOP•2y ago

yes

Pobiega•2y ago

only the word "active" should give anything back okay your method name should change it should be GetEmployeesByStatus(string status) in that case since its not JUST active employees anymore the query could be something like...

var active = status.Equals("active", StringComparison.OrdinalIgnoreCase);
return _context.Employees.Where(x => x.IsActive == active).ToList();

var active = status.Equals("active", StringComparison.OrdinalIgnoreCase);
return _context.Employees.Where(x => x.IsActive == active).ToList();

but even this is a bit weird if you only have 2 statuses (inactive/active) then you shouldnt be using a string discriminator a bool is fine, or maybe an enum

dragzunOP•2y ago

I know, thing is, on the section I am working at the moment, I only use Active Employees, but for other section, I will be using both types.

Pobiega•2y ago

thats fine, but then your method should be GetEmployeesByStatus(EmployeeStatus status) or GetEmployeesByActiveStatus(bool active) Whenever you pick a datatype, think of the number of valid values it has. In your case, its currently only 2 valid values: active and inactive (true or false). "cheese" is a valid value for a string, but its not a valid employee status

dragzunOP•2y ago

thank you, I tried your first approach suggestion and it worked, but I guess its not a great approach and I should probably go for one of the above

Pobiega•2y ago

yes. I'd probably introduce an enum the ASP.NET modelbinder handles them as strings and ints both, and works well

dragzunOP•2y ago

Thank you for all your help. Can I ask you one more thing ? its more of a refactoring suggestion of a method that works but I really don't like how it looks

Pobiega•2y ago

sure

dragzunOP•2y ago

basically I have a method which changes status of a client, again , from Active to Inactive, based on the clientId : this is how it looks public async Task<int> UpdateClientStatusAsync(int clientId) { var client = _context.Clients.First(w => w.ClientId == clientId);
if ( client.SysClientStatusId == (int)SysClientStatusType.Active) { client.SysClientStatusId = (int)SysClientStatusType.Inactive; } else if (client.SysClientStatusId == (int)SysClientStatusType.Inactive) { client.SysClientStatusId = (int)SysClientStatusType.Active; } await _context.SaveChangesAsync(); return client.ClientId; } I am using an enum for it but it looks kinda bad in my opinion

Pobiega•2y ago

$code btw

MODiX•2y ago

To post C# code type the following: ```cs // code here ``` Get an example by typing $codegif in chat If your code is too long, post it to: https://paste.mod.gg/

dragzunOP•2y ago

sorry

dragzunOP•2y ago

https://paste.mod.gg/uctrtnhlacir/0

BlazeBin - uctrtnhlacir

A tool for sharing your source code with the world!

Pobiega•2y ago

public async Task<int> UpdateClientStatusAsync(int clientId)
        {
          var client = _context.Clients.First(w => w.ClientId == clientId);

          if ( client.SysClientStatusId == (int)SysClientStatusType.Active)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Inactive;
            }

          else if (client.SysClientStatusId == (int)SysClientStatusType.Inactive)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Active;
            }

            await _context.SaveChangesAsync();

            return client.ClientId;
        }

public async Task<int> UpdateClientStatusAsync(int clientId)
        {
          var client = _context.Clients.First(w => w.ClientId == clientId);

          if ( client.SysClientStatusId == (int)SysClientStatusType.Active)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Inactive;
            }

          else if (client.SysClientStatusId == (int)SysClientStatusType.Inactive)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Active;
            }

            await _context.SaveChangesAsync();

            return client.ClientId;
        }

would have been fine too okay, why the (int) casts?

dragzunOP•2y ago

casue the status is an int in db public enum SysClientStatusType { Inactive = 435, Active = 788,
}

Pobiega•2y ago

okay. silly, but okay

dragzunOP•2y ago

so you re saying its fine as it is ? ah nvm, you were talking about the format 🙂

Pobiega•2y ago

public async Task<int> UpdateClientStatusAsync(int clientId)
{
    var client = await _context.Clients.FirstAsync(w => w.ClientId == clientId);

    client.SysClientStatusId = client.SysClientStatusId == (int)SysClientStatusType.Active
        ? (int)SysClientStatusType.Inactive
        : (int)SysClientStatusType.Active;

    await _context.SaveChangesAsync();

    return client.ClientId;
}

public async Task<int> UpdateClientStatusAsync(int clientId)
{
    var client = await _context.Clients.FirstAsync(w => w.ClientId == clientId);

    client.SysClientStatusId = client.SysClientStatusId == (int)SysClientStatusType.Active
        ? (int)SysClientStatusType.Inactive
        : (int)SysClientStatusType.Active;

    await _context.SaveChangesAsync();

    return client.ClientId;
}

is how I would write this also, you should be using the async versions of LINQ when accessing the context so FirstAsync, ToListAsync etc

dragzunOP•2y ago

got it, thanks a lot again 🙂 sorry to bother you again, actually I am happy I didnt close this yet. So basically the above method doesn't work as intended, since if a client is already inactive and i'm going to make a call to inactive it, it will actually activate the client. I was thinking I could add an enum status parameter to it, but I don't know how would I update the entity with that status ?

Pobiega•2y ago

well yes, thats exactly how it worked before too

 if ( client.SysClientStatusId == (int)SysClientStatusType.Active)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Inactive;
            }

          else if (client.SysClientStatusId == (int)SysClientStatusType.Inactive)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Active;
            }

 if ( client.SysClientStatusId == (int)SysClientStatusType.Active)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Inactive;
            }

          else if (client.SysClientStatusId == (int)SysClientStatusType.Inactive)
            {
                client.SysClientStatusId = (int)SysClientStatusType.Active;
            }

if active, set to inactive. else if inactive, set to active you never pass in a new status

dragzunOP•2y ago

yes, i know. Now I am considering either passing another parameter as an enum, or just create a dto with the client id and status id and have it as parameter. What would be your suggestion

Pobiega•2y ago

enum

dragzunOP•2y ago

I assume it has to be a different enum than the one that it is currently using?

Pobiega•2y ago

maybe? if you only want to activate/deactivate, yes if you want to allow any status to be set, no

dragzunOP•2y ago

thank you, i will give it a shot tomorrow. one more off topic question : if i have a dto with 5 properties, which is being used to by entity A , but I have another entity B that shares 3 of those properties, is there a way to use the same dto? Or just create a separate one with those 3 properties

Pobiega•2y ago

your could make DTO A extend DTO B so that B has the 3 props, and A inherits B and adds 2 more

dragzunOP•2y ago

ok, i got a bit confused, i am passing my new enum as a parameter but i don't know how can i compare the enums between each other ( the existing one and the one i added) https://paste.mod.gg/nxcfbjkygebz/0

BlazeBin - nxcfbjkygebz

A tool for sharing your source code with the world!

Pobiega•2y ago

well, you don't wait, does SysClientStatusType only have 2 valid values? I thought it had other statuses too if it only has two you can just reuse that one I guess, unless you think you will add new statuses further down the line and for the logic its very simple

public async Task<int> UpdateClientStatusAsync(int clientId, ClientStatusType status)
{
    var client = await _context.Clients.FirstOrDefaultAsync(w => w.ClientId == clientId);

    client.SysClientStatusId = status switch
    {
        ClientStatusType.Active => SysClientStatusType.Inactive,
        ClientStatusType.Inactive => SysClientStatusType.Active,
        _ => client.SysClientStatusId
    };

    await _context.SaveChangesAsync();

    return client.ClientId;
}

public async Task<int> UpdateClientStatusAsync(int clientId, ClientStatusType status)
{
    var client = await _context.Clients.FirstOrDefaultAsync(w => w.ClientId == clientId);

    client.SysClientStatusId = status switch
    {
        ClientStatusType.Active => SysClientStatusType.Inactive,
        ClientStatusType.Inactive => SysClientStatusType.Active,
        _ => client.SysClientStatusId
    };

    await _context.SaveChangesAsync();

    return client.ClientId;
}

dragzunOP•2y ago

thanks a lot, _ represents the entity, right ?

Pobiega•2y ago

no its the "any" value in a switch expression

dragzunOP•2y ago

Thanks a lot for all your help!

Gaming

Programming

✅ Pass status as parameter in controller