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

Support types without object equal operator ( extend test for DateTime, TimeSpan ) #3152

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

puschie286
Copy link
Contributor

Implement compare for value types.

Add test for DateTime and TimeSpan

~ set CultureInfo on test start
+ add test for DateTime
+ add test for TimeSpan
+ implement value type handling for expression compare
~ fix timespan test to use correct expecting values
@David-Moreira
Copy link
Contributor

So basically this is to access value types within structs? Is that it?
Can you add more tests for other value types within the struct just to make sure everything works fine?

@stsrki are we doing net6 only on 1.0.0? Maybe we can think about adding the new Date Types in .NET6 to the tests also.

@puschie286 why Activator over Expression.Default? Would the later not make more sense and be more performant in these cases? Could you please test?

+ add more corner test
~ replace activator with expression default
~ replace member.reflectedtype with item.type
@puschie286
Copy link
Contributor Author

@David-Moreira its for comparing struct that doesnt have a equal operator which accept objects.
Express.Constant( null ) is a object so it fail for some struct and Expression.Default / Activator always create a valid instance.
But im not 100% sure if this is how it works internaly - we use a lot of TimeSpan objects, which caused the expression to fail, so this is what i came up with^^

you are right with Expression.Default, thanks for pointing that out :)

replaced the reflection approach with your suggested expression approach

@puschie286 puschie286 changed the title Support value types ( extend test for DateTime, TimeSpan ) Support types without object equal operator ( extend test for DateTime, TimeSpan ) Nov 23, 2021
@David-Moreira
Copy link
Contributor

Way simpler change, makes more sense. If all tests are passing, LGTM.
(Do take note, that you're merging this in for 1.0.0, this change would only be live in a few months.)

@stsrki wanna consider this for 0.9.5? Seems worthy honestly.

@stsrki
Copy link
Collaborator

stsrki commented Nov 24, 2021

@David-Moreira Yeah, it is not breaking any API so it can be done as part of 095. LGTM

@stsrki stsrki changed the base branch from master to rel-0.9.5 November 24, 2021 06:31
@stsrki stsrki mentioned this pull request Nov 24, 2021
@stsrki stsrki merged commit ea5e6d4 into Megabit:rel-0.9.5 Nov 24, 2021
stsrki added a commit that referenced this pull request Nov 25, 2021
* Extract hex digits cache regex (#3140)

* Start With Benchmark

* Add ThemeGenerator for baseline benchmark

* ExtractHexDigits cache REGEX

* Methods Benchmarked + Tests

* Theme Generator : Refactor ExtractHexDigits

* Add test making sure ThemeGenerator is still logically equivalent to previous version.

* Run format

* Rename IsHexDigit

Co-authored-by: Mladen Macanovic <[email protected]>

* Fix Table THead Query Selector to target only first THead for FixedHeader and Resize features. (#3155)

* Support types without object equal operator ( extend test for DateTime, TimeSpan ) (#3152)

* Add test for DateTime and TimeSpan

~ set CultureInfo on test start
+ add test for DateTime
+ add test for TimeSpan

* Value type comparison

+ implement value type handling for expression compare
~ fix timespan test to use correct expecting values

* add more test & simplify

+ add more corner test
~ replace activator with expression default
~ replace member.reflectedtype with item.type

* Fix typos in docs (#3164)

* Mark snackbarInfos as readonly

* Fix unused snackbar title parameter

* Override ReleaseResources

* Fix missing comment param

Co-authored-by: David <[email protected]>
Co-authored-by: puschie286 <[email protected]>
@puschie286 puschie286 deleted the support-timespan branch December 2, 2021 14:30
stsrki added a commit that referenced this pull request Dec 8, 2021
* Unused vars (#3165)

* Extract hex digits cache regex (#3140)

* Start With Benchmark

* Add ThemeGenerator for baseline benchmark

* ExtractHexDigits cache REGEX

* Methods Benchmarked + Tests

* Theme Generator : Refactor ExtractHexDigits

* Add test making sure ThemeGenerator is still logically equivalent to previous version.

* Run format

* Rename IsHexDigit

Co-authored-by: Mladen Macanovic <[email protected]>

* Fix Table THead Query Selector to target only first THead for FixedHeader and Resize features. (#3155)

* Support types without object equal operator ( extend test for DateTime, TimeSpan ) (#3152)

* Add test for DateTime and TimeSpan

~ set CultureInfo on test start
+ add test for DateTime
+ add test for TimeSpan

* Value type comparison

+ implement value type handling for expression compare
~ fix timespan test to use correct expecting values

* add more test & simplify

+ add more corner test
~ replace activator with expression default
~ replace member.reflectedtype with item.type

* Fix typos in docs (#3164)

* Mark snackbarInfos as readonly

* Fix unused snackbar title parameter

* Override ReleaseResources

* Fix missing comment param

Co-authored-by: David <[email protected]>
Co-authored-by: puschie286 <[email protected]>

* Revert "Unused vars (#3165)" (#3167)

This reverts commit 13c9004.

* added czech translations for components

Added czech tranlations for data grid , file edit , time picker and date picker components

* Added translations for carousel and color picker

Added czech translations for carousel and color picker

Co-authored-by: Mladen Macanović <[email protected]>
Co-authored-by: David <[email protected]>
Co-authored-by: puschie286 <[email protected]>
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.

3 participants