Parsing string to Integer with try and catch???
-
[Edit] I know it is not a crime, but I had to fight against inconsistent variable names and bit of spaghetti code. But the most important thing is: This piece of code has to parse a huge amount of data (up to 500000 rows with parameters and so on) so you remember if it needs 50 or 4 seconds to parse the parameter values (overall time). [Edit end] I found this piece of code in a project which I had to expand. Notice the guy who wrote the code was at the end of his apprenticeship with already 4 years experience in coding. :doh: :wtf:
public void LogFileBreakDown(string channel, string channelnumber)
{
int channelnumber = 0;try
{
channelnumber = Int32.Parse(channelnumber);
}
catch(Exception ex)
{
channelnumber = 0;
}//Removed some specific code
return;
}I replaced Parse with TryParse -> and the log file was loading about 30 times faster then before. [EDIT: Added the new source as it is now]
public string Aufschluesselung_sys(String logFileLineText, String channelNumber, String channelName, bool showWarnings, ref string warnings, CultureInfo messageLanguage)
{
string sName, sID, sSplit = logFileLineText;
string[] mySplit = sSplit.Split(new Char[] { '>', '<', '"' });
string ctrlCommonSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlCommonSpecificationXml"]);
string cryptoModuleSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCryptoModuleSpecificationXml"]);
string ctrlDcSlaveSpecificationXml = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlDcSlaveSpecificationXml"]);
string ctrlIseSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlISESpecificationXml"]);XmlDocument XmlAufschluesselung = new XmlDocument(); int iID; if (mySplit.Count() >= 2) { if (Int32.TryParse((mySplit\[1\]), out iID)) { //XML File öffnen welches der gewünschte Wert enthält
Was this taken from old .NET 1/1.1 code though? TryParse wasn't introduced until .NET 2.
*pre-emptive celebratory nipple tassle jiggle* - Sean Ewington
"Mind bleach! Send me mind bleach!" - Nagy Vilmos
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier
-
Was this taken from old .NET 1/1.1 code though? TryParse wasn't introduced until .NET 2.
*pre-emptive celebratory nipple tassle jiggle* - Sean Ewington
"Mind bleach! Send me mind bleach!" - Nagy Vilmos
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier
It was made with .Net Framework 4.0, So TryParse was definately avaible. I didn't even had to change the target framework specification in the VS Setup.... :sigh:
-
[Edit] I know it is not a crime, but I had to fight against inconsistent variable names and bit of spaghetti code. But the most important thing is: This piece of code has to parse a huge amount of data (up to 500000 rows with parameters and so on) so you remember if it needs 50 or 4 seconds to parse the parameter values (overall time). [Edit end] I found this piece of code in a project which I had to expand. Notice the guy who wrote the code was at the end of his apprenticeship with already 4 years experience in coding. :doh: :wtf:
public void LogFileBreakDown(string channel, string channelnumber)
{
int channelnumber = 0;try
{
channelnumber = Int32.Parse(channelnumber);
}
catch(Exception ex)
{
channelnumber = 0;
}//Removed some specific code
return;
}I replaced Parse with TryParse -> and the log file was loading about 30 times faster then before. [EDIT: Added the new source as it is now]
public string Aufschluesselung_sys(String logFileLineText, String channelNumber, String channelName, bool showWarnings, ref string warnings, CultureInfo messageLanguage)
{
string sName, sID, sSplit = logFileLineText;
string[] mySplit = sSplit.Split(new Char[] { '>', '<', '"' });
string ctrlCommonSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlCommonSpecificationXml"]);
string cryptoModuleSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCryptoModuleSpecificationXml"]);
string ctrlDcSlaveSpecificationXml = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlDcSlaveSpecificationXml"]);
string ctrlIseSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlISESpecificationXml"]);XmlDocument XmlAufschluesselung = new XmlDocument(); int iID; if (mySplit.Count() >= 2) { if (Int32.TryParse((mySplit\[1\]), out iID)) { //XML File öffnen welches der gewünschte Wert enthält
How did he get it to compile?
A local variable named 'channelnumber' cannot be declared in this scope because it would give a different meaning to 'channelnumber', which is already used in a 'parent or current' scope to denote something else
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
-
How did he get it to compile?
A local variable named 'channelnumber' cannot be declared in this scope because it would give a different meaning to 'channelnumber', which is already used in a 'parent or current' scope to denote something else
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
Yes, This was a writing mistake by myself. The scond variables name should be "id". Sorry for that.
-
[Edit] I know it is not a crime, but I had to fight against inconsistent variable names and bit of spaghetti code. But the most important thing is: This piece of code has to parse a huge amount of data (up to 500000 rows with parameters and so on) so you remember if it needs 50 or 4 seconds to parse the parameter values (overall time). [Edit end] I found this piece of code in a project which I had to expand. Notice the guy who wrote the code was at the end of his apprenticeship with already 4 years experience in coding. :doh: :wtf:
public void LogFileBreakDown(string channel, string channelnumber)
{
int channelnumber = 0;try
{
channelnumber = Int32.Parse(channelnumber);
}
catch(Exception ex)
{
channelnumber = 0;
}//Removed some specific code
return;
}I replaced Parse with TryParse -> and the log file was loading about 30 times faster then before. [EDIT: Added the new source as it is now]
public string Aufschluesselung_sys(String logFileLineText, String channelNumber, String channelName, bool showWarnings, ref string warnings, CultureInfo messageLanguage)
{
string sName, sID, sSplit = logFileLineText;
string[] mySplit = sSplit.Split(new Char[] { '>', '<', '"' });
string ctrlCommonSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlCommonSpecificationXml"]);
string cryptoModuleSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCryptoModuleSpecificationXml"]);
string ctrlDcSlaveSpecificationXml = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlDcSlaveSpecificationXml"]);
string ctrlIseSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlISESpecificationXml"]);XmlDocument XmlAufschluesselung = new XmlDocument(); int iID; if (mySplit.Count() >= 2) { if (Int32.TryParse((mySplit\[1\]), out iID)) { //XML File öffnen welches der gewünschte Wert enthält
-
He didn't know about TryParse. I don't think that's a crime, really, particularly if not much of that 4 years' experience was in recent .Net. What he's done is exactly the pattern that you had to use before, and still have to use in Java.
Dear Bob A good argument indeed. I edited my original post with more information because there came up the whole discussion with performance and "How user-friendly is the program" for us. And the program used to crahs because it hung up in the whole try-catch story, so at least then he should've been looking for a faster way, doesn't he ;)
-
He didn't know about TryParse. I don't think that's a crime, really, particularly if not much of that 4 years' experience was in recent .Net. What he's done is exactly the pattern that you had to use before, and still have to use in Java.
Well if he's expecting a conversion/parsing exception, he still shouldn't be doing that Pokemon thing ;) Swallowing exceptions can be very dangerous sometimes, especially with large codebases and large input files.
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
-
Well if he's expecting a conversion/parsing exception, he still shouldn't be doing that Pokemon thing ;) Swallowing exceptions can be very dangerous sometimes, especially with large codebases and large input files.
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
-
True in principle ... but in reality there's no other exception that's going to be thrown in there, so catching Exception instead of FormatException doesn't really offend me in this case.
OutOfMemoryException and ThreadAbortException come to mind... Again, it's a big, long process (OP mentioned 50 seconds).
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
-
[Edit] I know it is not a crime, but I had to fight against inconsistent variable names and bit of spaghetti code. But the most important thing is: This piece of code has to parse a huge amount of data (up to 500000 rows with parameters and so on) so you remember if it needs 50 or 4 seconds to parse the parameter values (overall time). [Edit end] I found this piece of code in a project which I had to expand. Notice the guy who wrote the code was at the end of his apprenticeship with already 4 years experience in coding. :doh: :wtf:
public void LogFileBreakDown(string channel, string channelnumber)
{
int channelnumber = 0;try
{
channelnumber = Int32.Parse(channelnumber);
}
catch(Exception ex)
{
channelnumber = 0;
}//Removed some specific code
return;
}I replaced Parse with TryParse -> and the log file was loading about 30 times faster then before. [EDIT: Added the new source as it is now]
public string Aufschluesselung_sys(String logFileLineText, String channelNumber, String channelName, bool showWarnings, ref string warnings, CultureInfo messageLanguage)
{
string sName, sID, sSplit = logFileLineText;
string[] mySplit = sSplit.Split(new Char[] { '>', '<', '"' });
string ctrlCommonSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlCommonSpecificationXml"]);
string cryptoModuleSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCryptoModuleSpecificationXml"]);
string ctrlDcSlaveSpecificationXml = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlDcSlaveSpecificationXml"]);
string ctrlIseSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlISESpecificationXml"]);XmlDocument XmlAufschluesselung = new XmlDocument(); int iID; if (mySplit.Count() >= 2) { if (Int32.TryParse((mySplit\[1\]), out iID)) { //XML File öffnen welches der gewünschte Wert enthält
I'd add logging of the original string if TryParse returns false. Maybe it's indicative of a problem with callers if they pass a non-parseable string (somebody sends in "eight" instead of "8"), where you can continue without problems, but you want to research the application and see who's passing the offending strings.
-
I'd add logging of the original string if TryParse returns false. Maybe it's indicative of a problem with callers if they pass a non-parseable string (somebody sends in "eight" instead of "8"), where you can continue without problems, but you want to research the application and see who's passing the offending strings.
Maybe you are right, but the input is a machine-generated log file of a medical instrument, so there is no chance for a user to input wrong data. Further, it is possible that a line of the log file contains no ID, so it isn't very useful to search for any descriptions of the ID (which the program does when a Id was found)...
I am not french.
-
Yes, This was a writing mistake by myself. The scond variables name should be "id". Sorry for that.
Marco Bertschi wrote:
The scond variables name should be "id".
That was the first thing I saw, the second was the code didn't do anything. Third, I learned something. I had no idea Parse was slower than TryParse and I wouldn't have found out about it because the user is either a devious B or an idiot and won't put a number where asked to do so, so I wouldn't use Parse a second later than when I found out about TryParse. I also thought that a parsing routine should be as fast for both. Then I thought, it wouldn't be that fast if Parse was hitting exceptions, it gets caught miles from where it blew up and takes a while to get back to doing its job. Was it slow because it was blowing up or is there some intrinsic problem with the routine? (Besides being willing to blow up at the drop of a "h" at.) "Doh", I shouldn't type while thinking has gone out the window. Forget everything above, I probably have forgotten it already.
-
Marco Bertschi wrote:
The scond variables name should be "id".
That was the first thing I saw, the second was the code didn't do anything. Third, I learned something. I had no idea Parse was slower than TryParse and I wouldn't have found out about it because the user is either a devious B or an idiot and won't put a number where asked to do so, so I wouldn't use Parse a second later than when I found out about TryParse. I also thought that a parsing routine should be as fast for both. Then I thought, it wouldn't be that fast if Parse was hitting exceptions, it gets caught miles from where it blew up and takes a while to get back to doing its job. Was it slow because it was blowing up or is there some intrinsic problem with the routine? (Besides being willing to blow up at the drop of a "h" at.) "Doh", I shouldn't type while thinking has gone out the window. Forget everything above, I probably have forgotten it already.
KP Lee It does something, but I removed the specific code under the Parsing part. It was not slow because it threw an exception. It was slow because it threw about 500 exceptions during the import of one single file. However, your are right when you say that only a complete moron of a user would put a string instead of a number when a number is required. To complete the explanation, I added the whole source code to the original post.
-
Was this taken from old .NET 1/1.1 code though? TryParse wasn't introduced until .NET 2.
*pre-emptive celebratory nipple tassle jiggle* - Sean Ewington
"Mind bleach! Send me mind bleach!" - Nagy Vilmos
CodeStash - Online Snippet Management | My blog | MoXAML PowerToys | Mole 2010 - debugging made easier
Which is why I wrote my own utility method, TryParseInt, back in 2001... The method was based on the primary school teaching "4321" = 1 + 2*10 + 3*100 + 4*1000. An ad-hoc attempt to recreate it (although it's kinda pointless now)
static public bool TryParseInt(string s, out int n) { int baseValue = 1; n = 0; try { for (int pos = s.Length; pos >= 0; pos--) { char c = s\[pos\]; if (c == '-') { if (pos == 0) n \*= -1; else return false; } else if (c <= '0' || c >= '9') return false; n += baseValue \* (int)(c - '0'); } } catch (OverflowException) { return false; } return true; }
Funnily enough, when int.Parse was introduced with FW 2.0, I tested a little for fun, expecting the FW method to far and away outperform my simplistic method. But instead it didn't perform nearly as well as my simplistic method. Of course mine didn't bother about cultures or thousand separators, knew nothing about number formats, only worked for "invariant" (i.e. US!) culture, and even fell back on a try-catch to handle overflow. Still, for our use - where we'd interpret a string as one thing if it was a valid int, otherwise as soemthing else - this worked very well. :)
-
Marco Bertschi wrote:
The scond variables name should be "id".
That was the first thing I saw, the second was the code didn't do anything. Third, I learned something. I had no idea Parse was slower than TryParse and I wouldn't have found out about it because the user is either a devious B or an idiot and won't put a number where asked to do so, so I wouldn't use Parse a second later than when I found out about TryParse. I also thought that a parsing routine should be as fast for both. Then I thought, it wouldn't be that fast if Parse was hitting exceptions, it gets caught miles from where it blew up and takes a while to get back to doing its job. Was it slow because it was blowing up or is there some intrinsic problem with the routine? (Besides being willing to blow up at the drop of a "h" at.) "Doh", I shouldn't type while thinking has gone out the window. Forget everything above, I probably have forgotten it already.
Parse is not significantly slower than TryParse. (I suspect if you have a look with Reflector, you'll find it uses TryParse under the hood. Academically speaking I suppose it must in that case be slower, but in practice the difference would be insignificant at best.) But Parse in conjunction with try-catch is obviously a LOT slower than TryParse in conjunction with "if". And it's not necessary for the stack to be deep for this to be the case. Throwing an exception means allocating a new object (the exception) on the heap and this alone makes it far more costly. If you experiment a little with a simple console or winforms app you'll note that the first time you throw and catch an exception incurs a noticeable delay. After that it is a lot faster, but still thousands of times slower than returning a bool.
-
He didn't know about TryParse. I don't think that's a crime, really, particularly if not much of that 4 years' experience was in recent .Net. What he's done is exactly the pattern that you had to use before, and still have to use in Java.
I'd say "it depends". If the code was performance-critical, he could have written his own rudimentary method to avoid relying on exceptions to find out if a string represents a valid integer or not. He *should* know that throw-catch is slow, even if he's never seen a TryParse method before.
-
[Edit] I know it is not a crime, but I had to fight against inconsistent variable names and bit of spaghetti code. But the most important thing is: This piece of code has to parse a huge amount of data (up to 500000 rows with parameters and so on) so you remember if it needs 50 or 4 seconds to parse the parameter values (overall time). [Edit end] I found this piece of code in a project which I had to expand. Notice the guy who wrote the code was at the end of his apprenticeship with already 4 years experience in coding. :doh: :wtf:
public void LogFileBreakDown(string channel, string channelnumber)
{
int channelnumber = 0;try
{
channelnumber = Int32.Parse(channelnumber);
}
catch(Exception ex)
{
channelnumber = 0;
}//Removed some specific code
return;
}I replaced Parse with TryParse -> and the log file was loading about 30 times faster then before. [EDIT: Added the new source as it is now]
public string Aufschluesselung_sys(String logFileLineText, String channelNumber, String channelName, bool showWarnings, ref string warnings, CultureInfo messageLanguage)
{
string sName, sID, sSplit = logFileLineText;
string[] mySplit = sSplit.Split(new Char[] { '>', '<', '"' });
string ctrlCommonSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlCommonSpecificationXml"]);
string cryptoModuleSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCryptoModuleSpecificationXml"]);
string ctrlDcSlaveSpecificationXml = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlDcSlaveSpecificationXml"]);
string ctrlIseSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlISESpecificationXml"]);XmlDocument XmlAufschluesselung = new XmlDocument(); int iID; if (mySplit.Count() >= 2) { if (Int32.TryParse((mySplit\[1\]), out iID)) { //XML File öffnen welches der gewünschte Wert enthält
Hmm.. i see such silly mistakes being done all the time by fellow colleagues. This is due to the fact that, folks think they are good at their code and langauge grip. Inact, they should know that the art of programming has to be tinkered every day, then only such mistakes can be avoided. But hey, its the big EGO :x