|
Hi!
I wonder if this code is a correct way of using the IDisposable using keyword. Can I put the try-catch block inside it like this, and is it fine to insert a return inside the usage block? (the code is part of a method)
using(StreamReader reader = File.OpenText(fileName))
{
int lineIndex=0;
string line;
while((line=reader.ReadLine()) != null)
{
string[] fields = line.Split(';');
try
{
prodLst.Add(new Produkt(Int64.Parse(fields[0]), (ProdKategoriEnum)Int32.Parse(fields[1]), fields[2], Int32.Parse(fields[3]), Int32.Parse(fields[4])));
prodFileIndexDic.Add(Int64.Parse(fields[0]), lineIndex);
lineIndex++;
}
catch(FormatException e)
{
return false;
}
}
return true;
}
|
|
|
|
|
Entering a try..catch block is not free, in terms of overload. You may enclose your whole while loop in the try..catch block instead (you'll pay the overload only once).
And there's nothing wrong in returning a value inside the block; your StreamReader will be disposed anyway.
If you think it can be a problem, though, you just have to declare a boolean value before using statement, use it in place of your return statements in the using block, and return it at the end.
There are two kinds of people in the world: those who can extrapolate from incomplete data.
|
|
|
|
|
Yes, that's fine - just like a try...catch...finally block will always execute the code in the finally section, it doesn't matter how you exit the using block, the Dispose will still be called.
Personally, I'd probably put the try...catch around the whole loop just to look tidier, and put the return true outside the using so it's more obvious, but it doesn't make any real difference in practice.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
Not as an answer to your question, phil.o and OriginalGriff already covered that - but since you're apparently interested to learn about good practice:
When simply returning false in case of an exception, you'll never know why exactly your code failed. You should either log the exception message (and possibly its stacktrace) to a logfile or reconsider if it's not even better to just let your exception "bubble up" - meaning, your method would simply be a void method: Continuation of program execution means all went well, otherwise the exception will also abort the execution of the calling code scope. This is in most cases the best approach and saves you from checking boolean success-return values "everywhere". Only catch an exception if you're going to do something with it: Logging and rethrowing it (potentially wrapped as an InnerException into a custom exception) or actually doing something to remedy the error so that the program can continue normally.
Recursion: see Recursion.
|
|
|
|
|
As an implementation note one should keep in mind that dispose (which happens with using) doesn't insure that exceptions cannot be thrown.
Which is fine if you don't mind if an exception occurs on that last closing brace but if you are trying to create a fail safe method then it is inappropriate.
One should also consider what the impact is to the business functionality. So for example in the given sample code if one successfully read the file but then the close fails (in the dispose) then does the application (business functionality) really care?
|
|
|
|
|
Ok guys, thanks for your help.
|
|
|
|
|
Message Closed
modified 24-Mar-15 18:15pm.
|
|
|
|
|
You should never catch the general "Exception" type, except in the entry point to your application where you simply log the exception and exit.
In the OP's code, the try..catch block isn't needed, since there are TryParse methods which won't throw an exception if the input is invalid.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Message Closed
modified 24-Mar-15 18:15pm.
|
|
|
|
|
Gerry Schmitz wrote: You have no idea what he has "higher up".
Doesn't matter - swallowing every exception "just-in-case" is still a bad idea.
Gerry Schmitz wrote: You also can't seem to tell the difference between "Parse" and "TryParse".
What?! I suggest you try reading my comments again.
Gerry Schmitz wrote: You should "never" shoot your mouth off before thinking.
Talking to yourself is the first sign of madness.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Message Closed
modified 24-Mar-15 18:15pm.
|
|
|
|
|
Which will throw an exception when the input cannot be parsed.
Which is why I suggested using TryParse, which doesn't throw an exception.
Which would then mean that he wouldn't need a try..catch block to catch the exception from the Parse method.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Message Removed
modified 24-Mar-15 18:15pm.
|
|
|
|
|
As others have said, the code you've posted will work as expected.
However, you can avoid the try..catch block by using the TryParse methods. You should also check the result of line.Split(';') to avoid an IndexOutOfRangeException, check that the second field is a valid Enum value, and check that the dictionary doesn't already contain the key:
string[] fields = line.Split(';');
if (fields.Length < 5) return false;
long f0;
int f1, f3, f4;
if (!long.TryParse(fields[0], out f0)) return false;
if (!int.TryParse(fields[1], out f1)) return false;
if (!int.TryParse(fields[3], out f3)) return false;
if (!int.TryParse(fields[4], out f4)) return false;
if (prodFileIndexDic.ContainsKey(f0)) return false;
if (!Enum.IsDefined(typeof(ProdKategoriEnum), f1)) return false;
prodList.Add(new Produkt(f0, (ProdKategoriEnum)f1, fields[2], f3, f4));
prodFileIndexDic.Add(f0, lineIndex);
lineIndex++;
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
I have an application in C# which uses a SQLite database. In this database I have a table where I inserted a value encrypted with sha1, via sql query. Te problem is that I want to select that data and use it like:
cmd.CommandText = "Select * from accounts where (username=@username and password=sha1(@password));";
cmd.Parameters.AddWithValue("@password", password);
OR
cmd.CommandText = "Select * from accounts where (username=@username and password=@password);";
cmd.Parameters.AddWithValue("@password", sha1(password));
But from what I heard, I can't use sha1 like this, I have to use a function like this one, but it doesn't work:
string sha1(string password) {
byte[] byteArray = Encoding.UTF8.GetBytes(password);
return Convert.ToBase64String(sha1.ComputeHash(byteArray);
}
I receive error:
sha1(string) is a method, which is not valid in the given context
The other thing, I heard about KDF ( key definition function) which it seems to be a better encryption type but I don't really know how to use it. Can anybody help me with that?
modified 22-Mar-15 12:11pm.
|
|
|
|
|
|
Note he wrote "encryption" but meant "hashing" (sha1). I can't help him with that regarding SQLite but maybe you can add something to your answer.
Recursion: see Recursion.
|
|
|
|
|
It's ok, I want to use KDF instead, but I don't get anything of that code..I solve it with sha1..
|
|
|
|
|
|
in my forms i have combobox linked to sqlserver,the items in combobox is identity,two textbox and datetimepickers and button add ,when i press the button add at runtime i want to see the numbers of current adding information in combobox
please if you have any help answer me
think's a lot
|
|
|
|
|
Member 11545313 wrote: i have combobox linked to sqlserver
Uh, no you don't. You have a combobox bound to data that came from a database.
Member 11545313 wrote: the items in combobox is identity,two textbox and datetimepickers and button add
Quite impressive that you crammed all of that into a combobox.
Prehaps you have a combobox bound to the identity column of your data? Then you also have two textboxes, two datetime pickers and a button labeled "Add"?
Member 11545313 wrote: when i press the button add at runtime i want to see the numbers of current adding information in combobox
What do you mean by "numbers of current adding information"? That doesn't make sense at all.
|
|
|
|
|
I see your telepathy is only partially implemented!
Never underestimate the power of human stupidity
RAH
|
|
|
|
|
It's only an experimental Alpha right now. I don't plan to release a Beta version until Spring of next year.
|
|
|
|
|
Please tell me how to display a table of the Word document in DataGridView?
|
|
|
|
|
What have you tried?
Where are you stuck?
What do you need to know?
Have you read the table from the word file?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
