Search code examples
c#asp.netreadability

C# is there a nicer way of writing this?


int uploadsID;
int pageNumber;
int x;
int y;
int w;
int h;

bool isValidUploadID = int.TryParse(context.Request.QueryString["uploadID"], out uploadsID);
bool isValidPage = int.TryParse(context.Request.QueryString["page"], out pageNumber);
bool isValidX = int.TryParse(context.Request.QueryString["x"], out x);
bool isValidY = int.TryParse(context.Request.QueryString["y"], out y);
bool isValidW = int.TryParse(context.Request.QueryString["w"], out w);
bool isValidH = int.TryParse(context.Request.QueryString["h"], out h);

if (isValidUploadID && isValidPage && isValidX && isValidY & isValidW & isValidH)
{

This is an ajax handler, checking all passed params are OK. Is this considered bad, and is there a better way to write this, or is it not that important?


Solution

  • Assuming you're not going to use the individual bool variables elsewhere, you could write that as:

    int uploadsID, pageNumber, x, y, w, h;
    if (int.TryParse(context.Request.QueryString["uploadID"], out uploadsID) &&
        int.TryParse(context.Request.QueryString["page"], out pageNumber) &&
        int.TryParse(context.Request.QueryString["x"], out x) &&
        int.TryParse(context.Request.QueryString["y"], out y) &&
        int.TryParse(context.Request.QueryString["w"], out w) &&
        int.TryParse(context.Request.QueryString["h"], out h))
    {
    }
    

    You may want to extract out int.TryParse(context.Request.QueryString[name], out variable into a separate method, leaving you with something like:

    int uploadsID, pageNumber, x, y, w, h;
    if (TryParseContextInt32("uploadID", out uploadsID) &&
        TryParseContextInt32("page", out pageNumber) &&
        TryParseContextInt32("x", out x) &&
        TryParseContextInt32("y", out y) &&
        TryParseContextInt32("w", out w) &&
        TryParseContextInt32("h", out h))
    {
    }
    

    Alternatively, you could encapsulate all this context data into a new type with a TryParse method, so you'd have something like:

    PageDetails details;
    if (PageDetails.TryParse(context.Request.QueryString))
    {
        // Now access details.Page, details.UploadID etc
    }
    

    That's obviously more work, but I think it would make the code cleaner.