Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(type-extensions): add support for converting additional numeric types #250

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

dmitrii-kiselev
Copy link
Contributor

@dmitrii-kiselev dmitrii-kiselev commented Sep 28, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows Conventional Commits
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no API changes)
  • Other... Please describe:

What is the current behavior?

When using the GetFieldValue method, the following exceptions throw:

If the value in the database is stored as System.Int64 and the target type is System.SByte, this will result in an exception:

Unable to cast object of type 'System.Int64' to type 'System.SByte'.
   at EFCoreSecondLevelCacheInterceptor.EFTableRowsDataReader.GetFieldValue[T](Int32 ordinal) in EFTableRowsDataReader.cs:line 603

If the value in the database is stored as System.Int64 and the target type is System.UInt16, this will result in an exception:

Unable to cast object of type 'System.Int64' to type 'System.UInt16'.
   at EFCoreSecondLevelCacheInterceptor.EFTableRowsDataReader.GetFieldValue[T](Int32 ordinal) in EFTableRowsDataReader.cs:line 603

If the value in the database is stored as System.Int64 and the target type is System.Single, this will result in an exception:

Unable to cast object of type 'System.Int64' to type 'System.Single'.
   at EFCoreSecondLevelCacheInterceptor.EFTableRowsDataReader.GetFieldValue[T](Int32 ordinal) in EFTableRowsDataReader.cs:line 603

If the value in the database is stored as System.Int64 and the target type is System.Double, this will result in an exception:

Unable to cast object of type 'System.Int64' to type 'System.Double'.
   at EFCoreSecondLevelCacheInterceptor.EFTableRowsDataReader.GetFieldValue[T](Int32 ordinal) in EFTableRowsDataReader.cs:line 603

If the value in the database is stored as System.Int64 and the target type is System.Decimal, this will result in an exception:

Unable to cast object of type 'System.Int64' to type 'System.Decimal'.
   at EFCoreSecondLevelCacheInterceptor.EFTableRowsDataReader.GetFieldValue[T](Int32 ordinal) in EFTableRowsDataReader.cs:line 603

The same applies if the conversion should happen vice versa.

What is the new behavior?

Conversion without exceptions.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This issue was discovered through unit tests.

public void IsNumber_ShouldReturnExpectedResult(Type type, bool expected)

public void GetFieldValue_ThrowsInvalidCastException_WhenValueIsNotNumber(object value)

This happens because the IsNumber method in the TypeExtensions class supports a limited set of numeric types:

The change adds the following types:

  • sbyte
  • ushort
  • float
  • double
  • decimal

As an example, we can take SqliteValueReader, which works correctly in these cases:
https://github.com/dotnet/efcore/blob/a0443c1460e33f76094a30dc5ae0524743ed02f5/src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs#L223

https://github.com/dotnet/efcore/blob/a0443c1460e33f76094a30dc5ae0524743ed02f5/src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs#L248

https://github.com/dotnet/efcore/blob/a0443c1460e33f76094a30dc5ae0524743ed02f5/src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs#L320

https://github.com/dotnet/efcore/blob/a0443c1460e33f76094a30dc5ae0524743ed02f5/src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs#L315

https://github.com/dotnet/efcore/blob/a0443c1460e33f76094a30dc5ae0524743ed02f5/src/Microsoft.Data.Sqlite.Core/SqliteValueReader.cs#L310

Best Regards,
Dmitrii Kiselev

…types

This commit adds support for converting additional numeric types:
- `sbyte`
- `ushort`
- `float`
- `double`
- `decimal`
Copy link

what-the-diff bot commented Sep 28, 2024

PR Summary

  • Introduction of SByteType and UShortType
    Expanded the system's type handling capability by adding support for sbyte and ushort types. This means our system can now process these categories of data more efficiently.

  • Enhancements to IsNumber method
    Upgraded this function to include checks for additional data types (SByteType, UShortType, FloatType, DoubleType, and DecimalType). This improves the system's ability to identify whether any given input is a certain type of numerical data, ensuring more robust data identification.

  • Improvement in Testing Capability
    The test data in EFTableRowsDataReaderTests.cs has been expanded for sbyte, short, and ushort, strengthening our ability to verify that our system functions correctly with these types of data. In addition, the handling of decimal types in test cases has been improved.

  • Renaming of Test Method
    The test method GetFieldValue_ShouldReturnExpectedNumber was renamed to GetFieldValue_ShouldReturnExpectedDecimalNumber to make its purpose clearer, improving code readability and maintenance.

  • New Test for byte conversion
    A new test method GetFieldValue_ShouldReturnExpectedByteNumberFromChar has been added to confirm that the conversion from a character to a byte is being executed correctly, ensuring data accuracy in this type of conversion.

  • Updates in Exception Handling Tests
    The test cases have been fine-tuned to better support cases where a decimal number is being cast, increasing resilience in decimal handling.

  • Augmentation of TypeExtensionsTests.cs
    The testing conditions were revised to include sbyte, decimal, double, float, and ushort as supported types, enhancing the comprehensiveness of the system's testing.

@VahidN VahidN merged commit bca67fa into VahidN:master Sep 28, 2024
3 checks passed
@dmitrii-kiselev dmitrii-kiselev deleted the feature/numeric-types branch September 28, 2024 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants