Hey,

Currently the Int32.TryParse method calls the Int32.Parse method, and
catches the exceptions. That's not the best behaviour, since
Int32.TryParse is designed to avoid the performance problems associated
with exception handling.

The attached patch proposes an approach to keep the result of the
parsing as an enum, and based on it the methods decide whether they
throw an exception or not.

I wanted you to take a look at it, since we need to decide the best
solution for this before changing the TryParse/Parse methods in other
types.

Carlos.
Index: Int32.cs
===================================================================
--- Int32.cs	(revisión: 51973)
+++ Int32.cs	(copia de trabajo)
@@ -92,7 +92,7 @@
 		}
 #endif
 
-		internal static bool Parse (string s, bool tryParse, out int result)
+		internal static ParseResult Parse (string s, out int result)
 		{
 			int val = 0;
 			int len;
@@ -102,10 +102,7 @@
 			result = 0;
 
 			if (s == null)
-				if (tryParse)
-					return false;
-				else
-					throw new ArgumentNullException ("s");
+				return ParseResult.NullArgument;
 
 			len = s.Length;
 
@@ -117,10 +114,7 @@
 			}
 			
 			if (i == len)
-				if (tryParse)
-					return false;
-				else
-					throw new FormatException ();
+				return ParseResult.WrongFormat;
 
 			c = s [i];
 			if (c == '+')
@@ -145,28 +139,18 @@
 					if (Char.IsWhiteSpace (c)){
 						for (i++; i < len; i++){
 							if (!Char.IsWhiteSpace (s [i]))
-								if (tryParse)
-									return false;
-								else
-									throw new FormatException ();
+								return ParseResult.WrongFormat;
 						}
 						break;
 					} else
-						if (tryParse)
-							return false;
-						else
-							throw new FormatException ();
+						return ParseResult.WrongFormat;
 				}
 			}
 			if (!digits_seen)
-				if (tryParse)
-					return false;
-				else
-					throw new FormatException ();
+				return ParseResult.WrongFormat;
 
 			result = val;
-
-			return true;
+			return ParseResult.Success;
 		}
 
 		public static int Parse (string s, IFormatProvider fp)
@@ -179,7 +163,7 @@
 			return Parse (s, style, null);
 		}
 
-		internal static void CheckStyle (NumberStyles style)
+		internal static ParseResult CheckStyle (NumberStyles style)
 		{
 			if ((style & NumberStyles.AllowHexSpecifier) != 0) {
 				NumberStyles ne = style ^ NumberStyles.AllowHexSpecifier;
@@ -188,24 +172,37 @@
 				if ((ne & NumberStyles.AllowTrailingWhite) != 0)
 					ne ^= NumberStyles.AllowTrailingWhite;
 				if (ne != 0)
-					throw new ArgumentException (
-						"With AllowHexSpecifier only " + 
-						"AllowLeadingWhite and AllowTrailingWhite " + 
-						"are permitted.");
+					return ParseResult.WrongHexFormat;
 			}
+
+			return ParseResult.Success;
 		}
 		
-		internal static int JumpOverWhite (int pos, string s, bool excp)
+		internal static bool JumpOverWhite (ref int pos, string s)
 		{
+			return JumpOverWhite (ref pos, s, true);
+		}
+		
+		internal static bool JumpOverWhite (ref int pos, string s, bool report_error)
+		{
 			while (pos < s.Length && Char.IsWhiteSpace (s [pos]))
 				pos++;
 
-			if (excp && pos >= s.Length)
-				throw new FormatException ("Input string was not in the correct format.");
+			if (report_error && pos >= s.Length)
+				return false;
 
-			return pos;
+			return true;
 		}
 
+		internal static int JumpOverWhite (int pos, string s, bool exc)
+		{
+			int p = pos;
+			if (!JumpOverWhite (ref p, s, exc))
+				throw new FormatException ();
+
+			return p;
+		}
+
 		internal static void FindSign (ref int pos, string s, NumberFormatInfo nfi, 
 				      ref bool foundSign, ref bool negative)
 		{
@@ -275,22 +272,16 @@
 			return Char.IsDigit (e);
 		}
 		
-		internal static bool Parse (string s, NumberStyles style, IFormatProvider fp, bool tryParse, out int result)
+		internal static ParseResult Parse (string s, NumberStyles style, IFormatProvider fp, out int result)
 		{
+			ParseResult parse_result;
 			result = 0;
 
 			if (s == null)
-				if (tryParse)
-					return false;
-				else
-					throw new ArgumentNullException ();
+				return ParseResult.NullArgument;
 
 			if (s.Length == 0)
-				if (tryParse)
-					return false;
-				else
-					throw new FormatException ("Input string was not " + 
-											   "in the correct format.");
+				return ParseResult.WrongFormat;
 
 			NumberFormatInfo nfi;
 			if (fp != null) {
@@ -300,7 +291,9 @@
 			else
 				nfi = Thread.CurrentThread.CurrentCulture.NumberFormat;
 
-			CheckStyle (style);
+			parse_result = CheckStyle (style);
+			if (parse_result != ParseResult.Success)
+				return parse_result;
 
 			bool AllowCurrencySymbol = (style & NumberStyles.AllowCurrencySymbol) != 0;
 			bool AllowHexSpecifier = (style & NumberStyles.AllowHexSpecifier) != 0;
@@ -316,7 +309,8 @@
 			int pos = 0;
 
 			if (AllowLeadingWhite)
-				pos = JumpOverWhite (pos, s, true);
+				if (!JumpOverWhite (ref pos, s))
+					return ParseResult.WrongFormat;
 
 			bool foundOpenParentheses = false;
 			bool negative = false;
@@ -331,20 +325,14 @@
 						 // even when NumberFormatInfo.NumberNegativePattern != 0!!!
 				pos++;
 				if (AllowLeadingWhite)
-					pos = JumpOverWhite (pos, s, true);
+					if (!JumpOverWhite (ref pos, s))
+						return ParseResult.WrongFormat;
 
 				if (s.Substring (pos, nfi.NegativeSign.Length) == nfi.NegativeSign)
-					if (tryParse)
-						return false;
-					else
-						throw new FormatException ("Input string was not in the correct " +
-												   "format.");
+					return ParseResult.WrongFormat;
+				
 				if (s.Substring (pos, nfi.PositiveSign.Length) == nfi.PositiveSign)
-					if (tryParse)
-						return false;
-					else
-						throw new FormatException ("Input string was not in the correct " +
-												   "format.");
+					return ParseResult.WrongFormat;
 			}
 
 			if (AllowLeadingSign && !foundSign) {
@@ -352,12 +340,14 @@
 				FindSign (ref pos, s, nfi, ref foundSign, ref negative);
 				if (foundSign) {
 					if (AllowLeadingWhite)
-						pos = JumpOverWhite (pos, s, true);
+						if (!JumpOverWhite (ref pos, s))
+							return ParseResult.WrongFormat;
 					if (AllowCurrencySymbol) {
 						FindCurrency (ref pos, s, nfi,
 							      ref foundCurrency);
 						if (foundCurrency && AllowLeadingWhite)
-							pos = JumpOverWhite (pos, s, true);
+							if (!JumpOverWhite (ref pos, s))
+								return ParseResult.WrongFormat;
 					}
 				}
 			}
@@ -367,13 +357,15 @@
 				FindCurrency (ref pos, s, nfi, ref foundCurrency);
 				if (foundCurrency) {
 					if (AllowLeadingWhite)
-						pos = JumpOverWhite (pos, s, true);
+						if (!JumpOverWhite (ref pos, s))
+							return ParseResult.WrongFormat;
 					if (foundCurrency) {
 						if (!foundSign && AllowLeadingSign) {
 							FindSign (ref pos, s, nfi, ref foundSign,
 								  ref negative);
 							if (foundSign && AllowLeadingWhite)
-								pos = JumpOverWhite (pos, s, true);
+								if (!JumpOverWhite (ref pos, s))
+									return ParseResult.WrongFormat;
 						}
 					}
 				}
@@ -419,11 +411,7 @@
 					// Allows decimal point as long as it's only 
 					// followed by zeroes.
 					if (s [pos++] != '0')
-						if (tryParse)
-							return false;
-						else
-							throw new OverflowException ("Value too large or too " +
-														 "small.");
+						return ParseResult.Overflow;
 				}
 				else {
 					nDigits++;
@@ -436,21 +424,14 @@
 							(int) (s [pos++] - '0')
 							);
 					} catch (OverflowException) {
-						if (tryParse)
-							return false;
-						else
-							throw new OverflowException ("Value too large or too " +
-														 "small.");
+						return ParseResult.Overflow;
 					}
 				}
 			} while (pos < s.Length);
 
 			// Post number stuff
 			if (nDigits == 0)
-				if (tryParse)
-					return false;
-				else
-					throw new FormatException ("Input string was not in the correct format.");
+				return ParseResult.WrongFormat;
 
 			if (AllowExponent) 
 					FindExponent(ref pos, s);
@@ -460,7 +441,8 @@
 				FindSign (ref pos, s, nfi, ref foundSign, ref negative);
 				if (foundSign) {
 					if (AllowTrailingWhite)
-						pos = JumpOverWhite (pos, s, true);
+						if (!JumpOverWhite (ref pos, s))
+							return ParseResult.WrongFormat;
 					if (AllowCurrencySymbol)
 						FindCurrency (ref pos, s, nfi,
 							      ref foundCurrency);
@@ -472,7 +454,8 @@
 				FindCurrency (ref pos, s, nfi, ref foundCurrency);
 				if (foundCurrency) {
 					if (AllowTrailingWhite)
-						pos = JumpOverWhite (pos, s, true);
+						if (!JumpOverWhite (ref pos, s))
+							return ParseResult.WrongFormat;
 					if (!foundSign && AllowTrailingSign)
 						FindSign (ref pos, s, nfi, ref foundSign,
 							  ref negative);
@@ -480,72 +463,93 @@
 			}
 			
 			if (AllowTrailingWhite && pos < s.Length)
-				pos = JumpOverWhite (pos, s, false);
+				if (!JumpOverWhite (ref pos, s, false))
+					return ParseResult.WrongFormat;
 
 			if (foundOpenParentheses) {
 				if (pos >= s.Length || s [pos++] != ')')
-					if (tryParse)
-						return false;
-					else
-						throw new FormatException ("Input string was not in the correct " +
-												   "format.");
+					return ParseResult.WrongFormat;
 				if (AllowTrailingWhite && pos < s.Length)
-					pos = JumpOverWhite (pos, s, false);
+					if (!JumpOverWhite (ref pos, s, false))
+						return ParseResult.WrongFormat;
 			}
 
-			if (pos < s.Length && s [pos] != '\u0000') {
-				if (tryParse)
-					return false;
-				else
-					throw new FormatException ("Input string was not in the correct format.");
-			}
+			if (pos < s.Length && s [pos] != '\u0000')
+				return ParseResult.WrongFormat;
 			
 			if (!negative && !AllowHexSpecifier)
 				number = -number;
 
 			result = number;
 
-			return true;
+			return ParseResult.Success;
 		}
 
-		public static int Parse (string s) {
+		public static int Parse (string s) 
+		{
 			int res;
+			ParseResult result = Parse (s, out res);
+			if (result != ParseResult.Success)
+				throw GetParseException (result);
 
-			Parse (s, false, out res);
-
 			return res;
 		}
 
-		public static int Parse (string s, NumberStyles style, IFormatProvider fp) {
+		public static int Parse (string s, NumberStyles style, IFormatProvider fp) 
+		{
+			ParseResult result;
 			int res;
 
-			Parse (s, style, fp, false, out res);
+			result = Parse (s, style, fp, out res);
+			if (result != ParseResult.Success)
+				throw GetParseException (result);
 
 			return res;
 		}
 
 #if NET_2_0
-		public static bool TryParse (string s, out int result) {
-			try {
-				return Parse (s, true, out result);
-			}
-			catch (Exception) {
-				result = 0;
-				return false;
-			}
+		public static bool TryParse (string s, out int result) 
+		{
+			ParseResult parse_result = Parse (s, out result);
+			if (parse_result == ParseResult.Success)
+				return true;
+				
+			result = 0;
+			return false;
 		}
 
-		public static bool TryParse (string s, NumberStyles style, IFormatProvider provider, out int result) {
-			try {
-				return Parse (s, style, provider, true, out result);
-			}
-			catch (Exception) {
-				result = 0;
-				return false;
-			}
+		public static bool TryParse (string s, NumberStyles style, IFormatProvider provider, out int result) 
+		{
+			ParseResult parse_result = Parse (s, style, provider, out result);
+			if (parse_result == ParseResult.Success)
+				return true;
+
+			result = 0;
+			return false;
 		}
 #endif
+		internal static Exception GetParseException (ParseResult result)
+		{
+			switch (result) {
+				case ParseResult.NullArgument:
+					return new ArgumentNullException ();
+				case ParseResult.WrongFormat:
+					return new FormatException ("Input string was not in the correct format.");
+				case ParseResult.WrongHexFormat:
+					return new ArgumentException (
+						"With AllowHexSpecifier only " + 
+						"AllowLeadingWhite and AllowTrailingWhite " + 
+						"are permitted.");
+				case ParseResult.Overflow:
+					return new OverflowException ("Value too large or too small.");
+				default:
+					break;
+			}
 
+			// This shouldn't happen
+			return new Exception ("This shouldn't be reached");
+		}
+
 		public override string ToString ()
 		{
 			return NumberFormatter.FormatGeneral (new NumberFormatter.NumberStore (m_value));
_______________________________________________
Mono-devel-list mailing list
[email protected]
http://lists.ximian.com/mailman/listinfo/mono-devel-list

Reply via email to