Refactor Code

Accessing QueryString Values
by azamsharp on 5/3/2008 10:39:35 AM
Here is the sample code that I use to extract the values out of the query string. One of the things that I don't like is I am accessing the Request.QueryString["id"] two times. How can I improve it?
private void PopulateProducts()
{
if (Request.QueryString["id"] == null) return;

int id = Int32.Parse(Request.QueryString["id"]);

Response.Write(id); // or send to the database and fetch the thingy!
}
Refactor it!

you can use the C# null coalescing operator there.
by subdigital on 5/3/2008 11:43:57 AM
private void PopulateProducts()
{
int id = Int32.Parse(Request.QueryString["id"] ?? "-1");
if(id < 0) return;

Response.Write(id); //or db call to get products
}

good question, good effort. keep it up man. m sure there would be more better solution than mine. please make possible emailing of results :)
by MuhammadAdnan on 5/3/2008 11:46:23 AM
private void PopulateProducts()
{
object qid = Request.QueryString["id"];
if (qid == null) return;
int id = (int)qid;
Response.Write(id); // or send to the database and fetch the thingy!
}

I like Ben's solution! Simple and beautiful :)
by azamsharp on 5/3/2008 11:50:01 AM
private void PopulateProducts()
{
int id = Int32.Parse(Request.QueryString["id"] ?? "-1");
if(id < 0) return;

Response.Write(id); //or db call to get products
}

you can use the C# null coalescing operator there. by subdigital on 5/3/2008 11:43:57 AM .. solution is great but doesn't work when i give id less than 0 like id=-2 .. it returns :) which i guess it shouldn't ;)
by MuhammadAdnan on 5/3/2008 11:51:08 AM
private void PopulateProducts()
{
object qid = Request.QueryString["id"];
if (qid == null) return;
int id = (int)qid;
Response.Write(id); // or send to the database and fetch the thingy!
}

Hi Adnan, Well, I don't think you will have values that are 0 or less than 0. So, Ben's solution will work smoothly for 99.9% scenario's.
by azamsharp on 5/3/2008 12:17:52 PM

I'm assuming that that is an identity key, but you could easily change that to some other "magic" value.
by subdigital on 5/3/2008 12:19:46 PM

yes benn's solution is best IF value woule be more than 0. thats why i called it great :) good job man. thanks
by MuhammadAdnan on 5/3/2008 1:44:26 PM

Hi KeesDijk, Nice approach! Although I would still be inclined towards Ben's solution.
by azamsharp on 5/4/2008 10:25:14 AM

Hi azamsharp, Thanks. Just wondering why you would be inclined towards Ben's solution ? I Think : - using TryParse is more readable - having less exit points in a function is better, at the top exiting when an argument is invalid is fine by me, but if you can avoid it easily please do - Ben's solution doesn't work for negative id's
by KeesDijk on 5/4/2008 10:48:21 AM

Hi KeesDijk, You are right! Your approach is better! Thanks!
by azamsharp on 5/4/2008 7:42:05 PM
private void PopulateProducts()
{
int id;
if (!Int32.TryParse(Request.QueryString["id"], out id))
return;

// do something with the id

}

One more thing! if the ID is not an integer or not in correct format then what is the best way to deal with it. In the above code I am doing nothing so it will simply return. Would it be preferred to throw an exception and let the user know what is going on. Should the user even be bothered with this thing.
by azamsharp on 5/4/2008 7:43:41 PM

Referring to your query Azam if id is not in the correct format then tryparse method will return false. No need to handle or throw any exception. TryParse is preferred because of its this very utility. It never throws an exception.
by Ahsan on 10/24/2008 12:31:51 AM

Please log in to refactor the code! Login