Fwd: [mono-vb] A few fixes for Visual Basic Class

Jambunathan Jambunathan kjambunathan@novell.com
Sat, 06 Nov 2004 04:29:50 -0700


Ryan

My comments are in line -

> Index:
class/Microsoft.VisualBasic/Microsoft.VisualBasic/Conversion.cs
> ===================================================================
> RCS file:
/mono/mcs/class/Microsoft.VisualBasic/Microsoft.VisualBasic/Conversion.cs,v
> retrieving revision 1.12
> diff -u -r1.12 Conversion.cs
> ---
class/Microsoft.VisualBasic/Microsoft.VisualBasic/Conversion.cs	6
Sep 2004 11:23:11 -0000	1.12
> +++
class/Microsoft.VisualBasic/Microsoft.VisualBasic/Conversion.cs	27
Oct 2004 11:17:33 -0000
> @@ -700,7 +700,13 @@
>  				throw new ArgumentNullException
("Expression", 
>  					"Value cannot be null");
>  			}
> -		
> +			TypeCode TC = Type.GetTypeCode
(Expression.GetType()); 
> +			if (TC != TypeCode.String) {
> +				throw new ArgumentException(
> +				"Type of argument 'Expression' is '" + 
> +				Expression.GetType().FullName + 
> +				"', which can nt be converted to
numeric.");			
> +			}
>  			try {
>  				return Val (CastToString (Expression));

>  			} 


This is wrong. Why are you checking that expression is of type string
? The documentation says "If Expression is of type Object, its value
must be convertible to String or an ArgumentException error occurs."

With this fix, the following throws an exception in mono.

Dim I As Integer = 100
Dim D  As Double = Val (I)

> Index:
class/Microsoft.VisualBasic/Microsoft.VisualBasic/DateAndTime.cs
> ===================================================================
> RCS file:
/mono/mcs/class/Microsoft.VisualBasic/Microsoft.VisualBasic/DateAndTime.cs,v
> retrieving revision 1.13
> diff -u -r1.13 DateAndTime.cs
> ---
class/Microsoft.VisualBasic/Microsoft.VisualBasic/DateAndTime.cs	6
Sep 2004 13:47:44 -0000	1.13
> +++
class/Microsoft.VisualBasic/Microsoft.VisualBasic/DateAndTime.cs	27
Oct 2004 11:17:33 -0000
> @@ -509,10 +509,11 @@
>  		}
>  
>  		public static System.DateTime TimeValue (string
StringTime) 
> -		{ 
> -			try {
> -				return DateTime.MinValue +
DateTime.Parse(StringTime).TimeOfDay;
> -			} catch (FormatException exception) {
> +		{ 					
> +			try {				
> +				return DateTime.MinValue.Date +
DateTime.Parse(StringTime).TimeOfDay;
> +			} 
> +			catch (FormatException exception) {
>  				throw new InvalidCastException(null,
exception);
>  			}
>  		}
> @@ -576,9 +577,9 @@
>  			if (Weekday < 1 || Weekday > 7) {
>  				throw new ArgumentException();
>  			}
> -			Weekday += (int)FirstDayOfWeekValue;
> +			Weekday = Weekday -1  +
(int)FirstDayOfWeekValue;
>  			if (Weekday > 7) {
> -				Weekday -= 7;
> +				Weekday = Weekday -7;
>  			}
>  			if (Abbreviate) {
>  				return
CultureInfo.CurrentCulture.DateTimeFormat.GetAbbreviatedDayName((DayOfWeek)Weekday);


The above fix for WeekdayName is wrong. It either bahves badly or
throws exception in mono ! Try:

Console.WriteLine ("6 th day of week starting with Tuesday is {0}",
WeekdayName (6,,FirstDayOfWeek.Tuesday))
Console.WriteLine ("5 th day of week starting with Tuesday is  {0}",
WeekdayName (5,, FirstDayOfWeek.Tuesday))


> Index:
class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/ConversionTest.cs
> ===================================================================
> RCS file:
/mono/mcs/class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/ConversionTest.cs,v
> retrieving revision 1.2
> diff -u -r1.2 ConversionTest.cs
> ---
class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/ConversionTest.cs	24
Jun 2004 14:06:50 -0000	1.2
> +++
class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/ConversionTest.cs	27
Oct 2004 11:17:34 -0000
> @@ -684,19 +684,24 @@
>  				AssertEquals("#V06",
typeof(OverflowException), e.GetType());
>  				caughtException = true;
>  			}
> -
> -			AssertEquals("#V07", true, caughtException);
> +			finally {
> +				AssertEquals("#V07", true,
caughtException);
> +			}
>  
>  			caughtException = false;
>  
>  			try {
>  				Conversion.Val(typeof(int));
>  			}
> -			catch (Exception e) {
> -				AssertEquals("#V08",
typeof(ArgumentException), e.GetType());
> +			catch (Exception e) {
>  				caughtException = true;
> +				AssertEquals("#V08",
typeof(ArgumentException), e.GetType());
> +								
> +			}
> +			finally {
> +				AssertEquals("#V09", true,
caughtException);
>  			}
> -			AssertEquals("#V09", true, caughtException);
> +			
>  		}
>  	}
>  }


Will it not be more cleaner to use NUnit's [ExpectedException] ? 


> Index:
class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/DateAndTimeTest.cs
> ===================================================================
> RCS file:
/mono/mcs/class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/DateAndTimeTest.cs,v
> retrieving revision 1.5
> diff -u -r1.5 DateAndTimeTest.cs
> ---
class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/DateAndTimeTest.cs	24
Jun 2004 14:06:50 -0000	1.5
> +++
class/Microsoft.VisualBasic/Test/Microsoft.VisualBasic/DateAndTimeTest.cs	27
Oct 2004 11:17:34 -0000
> @@ -129,12 +129,16 @@
>  		public void TimeString() 
>  		{
>  			DateTime dtNow = DateTime.Now;
> +			System.Threading.Thread.Sleep((int) 2);
>  			TimeSpan tsNow = new TimeSpan(dtNow.Hour,
dtNow.Minute, dtNow.Second);
> +			System.Threading.Thread.Sleep((int) 2);
>  			string s = DateAndTime.TimeString;
>  			DateTime dtTest = DateTime.Parse(s);
>  			TimeSpan tsTest = new TimeSpan(dtTest.Hour,
dtTest.Minute, dtTest.Second);
>  			DateTime dtNow2 = DateTime.Now;
> +			System.Threading.Thread.Sleep((int) 2);
>  			TimeSpan tsNow2 = new TimeSpan(dtNow2.Hour,
dtNow2.Minute, dtNow2.Second);
> +			System.Threading.Thread.Sleep((int) 2);
>  			
>  			Assert("#TS01", tsTest.Ticks >= tsNow.Ticks);
>  			Assert("#TS02", tsNow2.Ticks >= tsTest.Ticks);
> @@ -286,12 +290,12 @@
>  				DateAndTime.TimeSerial(0, -1440, -1);
>  			}
>  			catch (Exception e) {
> -				AssertEquals("#TS01", e.GetType(),
typeof(ArgumentOutOfRangeException));
> +				AssertEquals("#TSL01",
typeof(ArgumentOutOfRangeException),  e.GetType());
>  				caughtException = true;
>  			}
> -			AssertEquals("#TS02", true, caughtException);
> +			AssertEquals("#TSL02", true, caughtException);
>  
> -			AssertEquals("#TS03", new DateTime(1, 1, 1, 1,
1, 1), DateAndTime.TimeSerial(1, 1, 1));
> +			AssertEquals("#TSL03", new DateTime(1, 1, 1, 1,
1, 1), DateAndTime.TimeSerial(1, 1, 1));
>  				
>  		}

By sleeping for a brief period aren't you making  the test less
stringent ?

>  		[Test]
> @@ -415,9 +436,9 @@
>  		{
>  			DateTime jan1 = new DateTime(2001, 1, 1, 1, 1,
1);
>  			AssertEquals("#WN01", "Tue",
>
-				DateAndTime.WeekdayName((int)jan1.DayOfWeek
+ 1, true, FirstDayOfWeek.Monday));
> +				DateAndTime.WeekdayName(1, true,
FirstDayOfWeek.Monday));
>  			AssertEquals("#WN02", "Tuesday",
>
-				DateAndTime.WeekdayName((int)jan1.DayOfWeek
+ 1, false, FirstDayOfWeek.Monday));
> +				DateAndTime.WeekdayName(1, false,
FirstDayOfWeek.Monday));
>  
>  			bool caughtException = false;
>  
> @@ -441,7 +462,7 @@
>  			}
>  			AssertEquals("#WN06", true, caughtException);
>  
> -			AssertEquals("#WN07", "Tuesday",
DateAndTime.WeekdayName((int)jan1.DayOfWeek + 1, false,
FirstDayOfWeek.Monday));
> +			AssertEquals("#WN07", "Tuesday",
DateAndTime.WeekdayName((int)jan1.DayOfWeek + 1, false,
FirstDayOfWeek.Sunday));
>  		}
>  	}
>  }


The first test case is wrong.Shouldn't it assert for Monday ?

On the formatting front:

1) If you are editing the file on a Windows box make sure you do a
    dos2unix before submitting the file.

2) Always include a ChangeLog entry when you submit patches. It
    will help us understand what you have done in the patch.

Please repost the refined patch.

The next time you submit a patch make sure you verify the
correctness. It will save us a lot of time :-)

Regards,
Jambunathan K.

>>> "Ryan L. Faircloth" <ryan@dss-i.com> 10/27/04 4:51 PM >>>

Some fixes were code some were test case issues. 

Let me know how these look if its the way you need them I will work on
more.