Skip to content

Commit

Permalink
Fixed TryGetField<T> to do a low level check instead of just wrapping…
Browse files Browse the repository at this point in the history 
… in try/catch blocks. Removed non generic TryGetField methods because they shouldn't be needed.
  • Loading branch information
@JoshClose
JoshClose committed Jul 7, 2011
1 parent d0814f6 commit 73bfc21
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 99 deletions.
2 changes: 1 addition & 1 deletion NuGet/CsvHelper.nuspec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<package xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<metadata xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd">
<id>CsvHelper</id>
<version>0.14.0</version>
<version>0.15.0</version>
<authors>Josh Close</authors>
<licenseUrl>http://www.opensource.org/licenses/ms-pl.html</licenseUrl>
<projectUrl>http://www.csvhelper.com</projectUrl>
Expand Down
72 changes: 63 additions & 9 deletions src/CsvHelper.Tests/CsvReaderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,61 @@ public void GetRecordsTest()
}

[TestMethod]
public void TryGetFieldInvalidTest()
public void TryGetFieldInvalidIndexTest()
{
var isHeaderRecord = true;
var data1 = new[] { "One", "Two" };
var data2 = new[] { "one", "two" };
var mockFactory = new MockFactory( MockBehavior.Default );
var parserMock = mockFactory.Create<ICsvParser>();
parserMock.Setup( m => m.Read() ).Returns( () =>
{
if( isHeaderRecord )
{
isHeaderRecord = false;
return data1;
}
return data2;
} );

var reader = new CsvReader( parserMock.Object );
reader.Read();

int field;
var got = reader.TryGetField( 0, out field );
Assert.IsFalse( got );
Assert.AreEqual( default( int ), field );
}

[TestMethod]
public void TryGetFieldInvalidNameTest()
{
var isHeaderRecord = true;
var data1 = new[] { "One", "Two" };
var data2 = new[] { "one", "two" };
var mockFactory = new MockFactory( MockBehavior.Default );
var parserMock = mockFactory.Create<ICsvParser>();
parserMock.Setup( m => m.Read() ).Returns( () =>
{
if( isHeaderRecord )
{
isHeaderRecord = false;
return data1;
}
return data2;
} );

var reader = new CsvReader( parserMock.Object );
reader.Read();

int field;
var got = reader.TryGetField( "One", out field );
Assert.IsFalse( got );
Assert.AreEqual( default( int ), field );
}

[TestMethod]
public void TryGetFieldInvalidConverterIndexTest()
{
var isHeaderRecord = true;
var data1 = new[] { "One", "Two" };
Expand All @@ -322,14 +376,14 @@ public void TryGetFieldInvalidTest()
var reader = new CsvReader( parserMock.Object );
reader.Read();

string field;
var got = reader.TryGetField( -1, out field );
int field;
var got = reader.TryGetField( 0, new GuidConverter(), out field );
Assert.IsFalse( got );
Assert.IsNull( field );
Assert.AreEqual( default( int ), field );
}

[TestMethod]
public void TryGetFieldInvalidStrictTest()
public void TryGetFieldInvalidConverterNameTest()
{
var isHeaderRecord = true;
var data1 = new[] { "One", "Two" };
Expand All @@ -346,13 +400,13 @@ public void TryGetFieldInvalidStrictTest()
return data2;
} );

var reader = new CsvReader( parserMock.Object ) { Configuration = { Strict = true } };
var reader = new CsvReader( parserMock.Object );
reader.Read();

string field;
var got = reader.TryGetField( -1, out field );
int field;
var got = reader.TryGetField( "One", new GuidConverter(), out field );
Assert.IsFalse( got );
Assert.IsNull( field );
Assert.AreEqual( default( int ), field );
}

[TestMethod]
Expand Down
108 changes: 36 additions & 72 deletions src/CsvHelper/CsvReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ public virtual string this[int index]
{
get
{
CheckDisposed();
CheckHasBeenRead();

return GetField( index );
}
}
Expand All @@ -120,6 +123,9 @@ public virtual string this[string name]
{
get
{
CheckDisposed();
CheckHasBeenRead();

return GetField( name );
}
}
Expand All @@ -131,6 +137,9 @@ public virtual string this[string name]
/// <returns>The raw field.</returns>
public virtual string GetField( int index )
{
CheckDisposed();
CheckHasBeenRead();

return currentRecord[index];
}

Expand All @@ -141,6 +150,9 @@ public virtual string GetField( int index )
/// <returns>The raw field.</returns>
public virtual string GetField( string name )
{
CheckDisposed();
CheckHasBeenRead();

var index = GetFieldIndex( name );
if( index < 0 )
{
Expand Down Expand Up @@ -211,46 +223,6 @@ public virtual T GetField<T>( string name, TypeConverter converter )
return (T)GetField( name, converter );
}

/// <summary>
/// Gets the raw field at index.
/// </summary>
/// <param name="index">The index of the field.</param>
/// <param name="field">The raw field.</param>
/// <returns>A value indicating if the get was successful.</returns>
public virtual bool TryGetField( int index, out string field )
{
try
{
field = GetField( index );
}
catch
{
field = default( string );
return false;
}
return true;
}

/// <summary>
/// Gets the raw field at name.
/// </summary>
/// <param name="name">The named index of the field.</param>
/// <param name="field">The raw field.</param>
/// <returns>A value indicating if the get was successful.</returns>
public virtual bool TryGetField( string name, out string field )
{
try
{
field = GetField( name );
}
catch
{
field = default( string );
return false;
}
return true;
}

/// <summary>
/// Gets the field converted to <see cref="Type"/> T at index.
/// </summary>
Expand All @@ -260,16 +232,11 @@ public virtual bool TryGetField( string name, out string field )
/// <returns>A value indicating if the get was successful.</returns>
public virtual bool TryGetField<T>( int index, out T field )
{
try
{
field = GetField<T>( index );
}
catch
{
field = default( T );
return false;
}
return true;
CheckDisposed();
CheckHasBeenRead();

var converter = TypeDescriptor.GetConverter( typeof( T ) );
return TryGetField( index, converter, out field );
}

/// <summary>
Expand All @@ -281,16 +248,11 @@ public virtual bool TryGetField<T>( int index, out T field )
/// <returns>A value indicating if the get was successful.</returns>
public virtual bool TryGetField<T>( string name, out T field )
{
try
{
field = GetField<T>( name );
}
catch
{
field = default( T );
return false;
}
return true;
CheckDisposed();
CheckHasBeenRead();

var converter = TypeDescriptor.GetConverter( typeof( T ) );
return TryGetField( name, converter, out field );
}

/// <summary>
Expand All @@ -304,15 +266,17 @@ public virtual bool TryGetField<T>( string name, out T field )
/// <returns>A value indicating if the get was successful.</returns>
public virtual bool TryGetField<T>( int index, TypeConverter converter, out T field )
{
try
{
field = GetField<T>( index, converter );
}
catch
CheckDisposed();
CheckHasBeenRead();

var rawField = currentRecord[index];
if(!converter.IsValid( rawField ))
{
field = default( T );
return false;
}

field = (T)GetField( index, converter );
return true;
}

Expand All @@ -327,16 +291,16 @@ public virtual bool TryGetField<T>( int index, TypeConverter converter, out T fi
/// <returns>A value indicating if the get was successful.</returns>
public virtual bool TryGetField<T>( string name, TypeConverter converter, out T field )
{
try
{
field = GetField<T>( name, converter );
}
catch
CheckDisposed();
CheckHasBeenRead();

var index = GetFieldIndex( name );
if( index == -1 )
{
field = default( T );
return false;
}
return true;
return TryGetField( index, converter, out field );
}

/// <summary>
Expand Down Expand Up @@ -445,7 +409,7 @@ protected virtual void CheckHasBeenRead()
/// <returns>The field converted to <see cref="Object"/>.</returns>
protected virtual object GetField( int index, TypeConverter converter )
{
return converter.ConvertFrom( currentRecord[index] );
return converter.ConvertFromInvariantString( currentRecord[index] );

This comment has been minimized.

Sign in to view

Copy link
@vcaraulean

vcaraulean Jul 25, 2011

Contributor

This modification is leading to #16.

When I'm change it back, all tests are green.
}

/// <summary>
Expand Down
16 changes: 0 additions & 16 deletions src/CsvHelper/ICsvReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,6 @@ public interface ICsvReader : IDisposable
/// <returns>The field converted to <see cref="Type"/> T.</returns>
T GetField<T>( string name, TypeConverter converter );

/// <summary>
/// Gets the raw field at index.
/// </summary>
/// <param name="index">The index of the field.</param>
/// <param name="field">The raw field.</param>
/// <returns>A value indicating if the get was successful.</returns>
bool TryGetField( int index, out string field );

/// <summary>
/// Gets the raw field at name.
/// </summary>
/// <param name="name">The named index of the field.</param>
/// <param name="field">The raw field.</param>
/// <returns>A value indicating if the get was successful.</returns>
bool TryGetField( string name, out string field );

/// <summary>
/// Gets the field converted to <see cref="Type"/> T at index.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/CsvHelper/Properties/AssemblyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@
//
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
[assembly: AssemblyVersion( "0.14.0.*" )]
[assembly: AssemblyVersion( "0.15.0.*" )]

0 comments on commit 73bfc21

Please sign in to comment.