Release 0.6.0 #7
Conversation
…ry* data types to properly resolve lists and dictionaries with different data types from the original collection. Fixed Unit tests for floatPTests that were failing. Added new unit tests for *ObservableResolverList* and *ObservableResolverDictionary* data types
📝 WalkthroughWalkthroughThe changes in this pull request include updates to the changelog, which details new features and fixes across multiple versions, specifically highlighting the introduction of new data types and enhancements to existing ones. Notable additions include Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
WalkthroughThe changes include improvements to data types for resolving lists and dictionaries, fixes to unit tests, and the addition of new unit tests for Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (8)
- CHANGELOG.md (1)
- Runtime/ObservableDictionary.cs (8)
- Runtime/ObservableList.cs (8)
- Tests/Editor/OberservableResolverListTest.cs (1)
- Tests/Editor/ObservableDictionaryTest.cs (3)
- Tests/Editor/ObservableListTest.cs (1)
- Tests/Editor/ObservableResolverDictionaryTest.cs (1)
- Tests/Editor/floatPTests.cs (2)
Files ignored due to filter (3)
- Tests/Editor/OberservableResolverListTest.cs.meta
- Tests/Editor/ObservableResolverDictionaryTest.cs.meta
- package.json
Files not summarized due to errors (1)
- Tests/Editor/ObservableDictionaryTest.cs (diff tokens exceeds limit)
Review comments generated (71)
- Review: 57
- LGTM: 14
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| - Added utility methods and extensions for Unity's Object and GameObject types, enhancing the codebase's functionality. | ||
| - Introduced a SerializableType struct for viewing, modifying, and saving types from the inspector, with serialization support and compatibility with filter attributes. |
There was a problem hiding this comment.
The description states that the assertions in the floatPTests class have been adjusted to cast float values explicitly to floatP. However, the code changes in this hunk are missing, so it's difficult to provide a thorough review. Please include the actual code changes for a more accurate review.
Changelog:
| - Added new *ObservableResolverList*, *ObservableResolverDictionary* & *ObservableResolverField* to allow to create observable types without referencing the collection directly | ||
| - Added Unit tests to all types | ||
| - Added Unit tests to all data types in the project |
There was a problem hiding this comment.
The changelog entry for version 0.6.0 mentions improvements to the ObservableResolverList and ObservableResolverDictionary data types. However, the actual changes made to these data types are not mentioned in the changelog. It would be helpful to provide more details about the specific improvements made to these data types.
Overall, please provide the missing code changes for a more accurate review of the logic, performance, and other aspects of the code.
| [TestFixture] | ||
| public class ObservableResolverListTest | ||
| { | ||
| private int _index = 0; | ||
| private IObservableResolverList<int, string> _list; | ||
| private IList<string> _mockList; |
There was a problem hiding this comment.
The ObservableResolverListTest class is missing a namespace declaration. Please add the appropriate namespace declaration for the test class.
+ using GameLoversEditor.DataExtensions.Tests;| public void AddOrigin_AddsValueToOriginList() | ||
| { | ||
| var value = "1"; | ||
|
|
||
| _list.AddOrigin(value); | ||
|
|
||
| _mockList.Received().Add(value); | ||
| } |
There was a problem hiding this comment.
The AddOrigin_AddsValueToOriginList test method looks good. It verifies that calling the AddOrigin method adds the value to the origin list. The assertion is made using the Received method of the _mockList object. This ensures that the Add method of the _mockList object is called with the expected value. Well done!
| [Test] | ||
| public void UpdateOrigin_UpdatesOriginList() | ||
| { | ||
| var value = "1"; | ||
|
|
||
| _list.AddOrigin(value); | ||
| _list.UpdateOrigin(value, _index); | ||
|
|
||
| _mockList.Received()[_index] = value; | ||
| } |
There was a problem hiding this comment.
The UpdateOrigin_UpdatesOriginList test method also looks good. It tests that calling the UpdateOrigin method updates the value at the specified index in the origin list. The assertion is made using the Received indexer of the _mockList object. This ensures that the value at the specified index is set to the expected value. Great job!
| public bool RemoveOrigin(TOrigin value) | ||
| { | ||
| _originList.Remove(value); | ||
|
|
||
| return base.Remove(_fromOrignResolver(value)); | ||
| } |
There was a problem hiding this comment.
The implementation of the RemoveOrigin method in the ObservableResolverList<T, TOrigin> class is incorrect. It should remove the value from the _originList before calling the base Remove method. Please update the implementation as follows:
+ public bool RemoveOrigin(TOrigin value)
+ {
+ _originList.Remove(value);
+ return base.Remove(_fromOrignResolver(value));
+ }| public void ClearOrigin() | ||
| { | ||
| _listResolver = listResolver; | ||
| _originList.Clear(); | ||
| base.Clear(); | ||
| } |
There was a problem hiding this comment.
The implementation of the ClearOrigin method in the ObservableResolverList<T, TOrigin> class is incorrect. It should clear the _originList before calling the base Clear method. Please update the implementation as follows:
+ public void ClearOrigin()
+ {
+ _originList.Clear();
+ base.Clear();
+ }| Assert.AreEqual(floatP.Zero, (floatP)0f); | ||
| Assert.AreEqual(-floatP.Zero, (floatP) (-0f)); | ||
| Assert.AreEqual(floatP.Zero, -floatP.Zero); | ||
| Assert.AreEqual(floatP.NaN, float.NaN); | ||
| Assert.AreEqual(floatP.One, 1f); | ||
| Assert.AreEqual(floatP.MinusOne, -1f); | ||
| Assert.AreEqual(floatP.PositiveInfinity, float.PositiveInfinity); | ||
| Assert.AreEqual(floatP.NegativeInfinity, float.NegativeInfinity); | ||
| Assert.AreEqual(floatP.Epsilon, float.Epsilon); | ||
| Assert.AreEqual(floatP.MaxValue, float.MaxValue); | ||
| Assert.AreEqual(floatP.MinValue, float.MinValue); | ||
| Assert.AreEqual(floatP.NaN, (floatP) float.NaN); | ||
| Assert.AreEqual(floatP.MinusOne, (floatP) (- 1f)); | ||
| Assert.AreEqual(floatP.PositiveInfinity, (floatP)float.PositiveInfinity); | ||
| Assert.AreEqual(floatP.NegativeInfinity, (floatP)float.NegativeInfinity); | ||
| Assert.AreEqual(floatP.Epsilon, (floatP)float.Epsilon); | ||
| Assert.AreEqual(floatP.MaxValue, (floatP)float.MaxValue); | ||
| Assert.AreEqual(floatP.MinValue, (floatP)float.MinValue); |
There was a problem hiding this comment.
The unit test Representation in the floatPTests class is comparing floatP values with float values, which will always fail. The floatP values need to be explicitly cast to float before comparison. Here's the fix:
- Assert.AreEqual(floatP.Zero, (floatP)0f);
+ Assert.AreEqual((float)floatP.Zero, 0f);
- Assert.AreEqual(-floatP.Zero, (floatP)(-0f));
+ Assert.AreEqual((float)-floatP.Zero, -0f);
- Assert.AreEqual(floatP.NaN, (floatP)float.NaN);
+ Assert.AreEqual((float)floatP.NaN, float.NaN);
- Assert.AreEqual(floatP.MinusOne, (floatP)(-1f));
+ Assert.AreEqual((float)floatP.MinusOne, -1f);
- Assert.AreEqual(floatP.PositiveInfinity, (floatP)float.PositiveInfinity);
+ Assert.AreEqual((float)floatP.PositiveInfinity, float.PositiveInfinity);
- Assert.AreEqual(floatP.NegativeInfinity, (floatP)float.NegativeInfinity);
+ Assert.AreEqual((float)floatP.NegativeInfinity, float.NegativeInfinity);
- Assert.AreEqual(floatP.Epsilon, (floatP)float.Epsilon);
+ Assert.AreEqual((float)floatP.Epsilon, float.Epsilon);
- Assert.AreEqual(floatP.MaxValue, (floatP)float.MaxValue);
+ Assert.AreEqual((float)floatP.MaxValue, float.MaxValue);
- Assert.AreEqual(floatP.MinValue, (floatP)float.MinValue);
+ Assert.AreEqual((float)floatP.MinValue, float.MinValue);| Assert.AreEqual(floatP.One + floatP.One, (floatP)2f); | ||
| Assert.AreEqual(floatP.One - floatP.One, (floatP)0f); | ||
| } |
There was a problem hiding this comment.
The unit test Addition in the floatPTests class is comparing floatP values with float values, which will always fail. The floatP values need to be explicitly cast to float before comparison. Here's the fix:
- Assert.AreEqual(floatP.One + floatP.One, (floatP)2f);
+ Assert.AreEqual((float)floatP.One + (float)floatP.One, 2f);
- Assert.AreEqual(floatP.One - floatP.One, (floatP)0f);
+ Assert.AreEqual((float)floatP.One - (float)floatP.One, 0f);| Assert.AreEqual(floatP.PositiveInfinity * floatP.Zero, (floatP) (float.PositiveInfinity * 0f)); | ||
| Assert.AreEqual(floatP.PositiveInfinity * (-floatP.Zero), (floatP)(float.PositiveInfinity * (-0f))); | ||
| Assert.AreEqual(floatP.PositiveInfinity * floatP.One, (floatP)(float.PositiveInfinity * 1f)); | ||
| Assert.AreEqual(floatP.PositiveInfinity * floatP.MinusOne, (floatP)(float.PositiveInfinity * -1f)); | ||
|
|
||
| Assert.AreEqual(floatP.NegativeInfinity * floatP.Zero, float.NegativeInfinity * 0f); | ||
| Assert.AreEqual(floatP.NegativeInfinity * (-floatP.Zero), float.NegativeInfinity * (-0f)); | ||
| Assert.AreEqual(floatP.NegativeInfinity * floatP.One, float.NegativeInfinity * 1f); | ||
| Assert.AreEqual(floatP.NegativeInfinity * floatP.MinusOne, float.NegativeInfinity * -1f); | ||
| Assert.AreEqual(floatP.NegativeInfinity * floatP.Zero, (floatP)(float.NegativeInfinity * 0f)); | ||
| Assert.AreEqual(floatP.NegativeInfinity * (-floatP.Zero), (floatP)(float.NegativeInfinity * (-0f))); | ||
| Assert.AreEqual(floatP.NegativeInfinity * floatP.One, (floatP)(float.NegativeInfinity * 1f)); | ||
| Assert.AreEqual(floatP.NegativeInfinity * floatP.MinusOne, (floatP)(float.NegativeInfinity * -1f)); | ||
|
|
||
| Assert.AreEqual(floatP.One * floatP.One, 1f); | ||
| Assert.AreEqual(floatP.One * floatP.One, (floatP)1f); |
There was a problem hiding this comment.
The unit test Multiplication in the floatPTests class is comparing floatP values with float values, which will always fail. The floatP values need to be explicitly cast to float before comparison. Here's the fix:
- Assert.AreEqual(floatP.PositiveInfinity * floatP.Zero, (floatP)(float.PositiveInfinity * 0f));
+ Assert.AreEqual((float)floatP.PositiveInfinity * (float)floatP.Zero, float.PositiveInfinity * 0f);
- Assert.AreEqual(floatP.PositiveInfinity * (-floatP.Zero), (floatP)(float.PositiveInfinity * (-0f)));
+ Assert.AreEqual((float)floatP.PositiveInfinity * (float)-floatP.Zero, float.PositiveInfinity * (-0f));
- Assert.AreEqual(floatP.PositiveInfinity * floatP.One, (floatP)(float.PositiveInfinity * 1f));
+ Assert.AreEqual((float)floatP.PositiveInfinity * (float)floatP.One, float.PositiveInfinity * 1f);
- Assert.AreEqual(floatP.PositiveInfinity * floatP.MinusOne, (floatP)(float.PositiveInfinity * -1f));
+ Assert.AreEqual((float)floatP.PositiveInfinity * (float)floatP.MinusOne, float.PositiveInfinity * -1f);
- Assert.AreEqual(floatP.NegativeInfinity * floatP.Zero, (floatP)(float.NegativeInfinity * 0f));
+ Assert.AreEqual((float)floatP.NegativeInfinity * (float)floatP.Zero, float.NegativeInfinity * 0f);
- Assert.AreEqual(floatP.NegativeInfinity * (-floatP.Zero), (floatP)(float.NegativeInfinity * (-0f)));
+ Assert.AreEqual((float)floatP.NegativeInfinity * (float)-floatP.Zero, float.NegativeInfinity * (-0f));
- Assert.AreEqual(floatP.NegativeInfinity * floatP.One, (floatP)(float.NegativeInfinity * 1f));
+ Assert.AreEqual((float)floatP.NegativeInfinity * (float)floatP.One, float.NegativeInfinity * 1f);
- Assert.AreEqual(floatP.NegativeInfinity * floatP.MinusOne, (floatP)(float.NegativeInfinity * -1f));
+ Assert.AreEqual((float)floatP.NegativeInfinity * (float)floatP.MinusOne, float.NegativeInfinity * -1f);Please review these changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Outside diff range and nitpick comments (20)
Tests/Editor/OberservableResolverListTest.cs (3)
17-28: LGTM: Proper setup, with a minor suggestionThe setup method and field declarations are well-structured and follow best practices for unit testing. The use of NSubstitute for mocking and the initialization of
ObservableResolverListwith conversion functions are appropriate.Consider initializing
_indexin theSetUpmethod for consistency and to ensure a fresh state for each test:private int _index = 0; private IObservableResolverList<int, string> _list; private IList<string> _mockList; [SetUp] public void SetUp() { + _index = 0; _mockList = Substitute.For<IList<string>>(); _list = new ObservableResolverList<int, string>(_mockList, origin => int.Parse(origin), value => value.ToString()); }
40-60: LGTM: Effective tests for UpdateOrigin and RemoveOrigin methods, with a suggestionBoth test methods correctly verify the behavior of their respective
ObservableResolverListmethods. The assertions are appropriate and check the expected interactions with the mock list.Consider parameterizing the
UpdateOrigin_UpdatesOriginListtest to check multiple index values:[Test] [TestCase(0)] [TestCase(1)] [TestCase(5)] public void UpdateOrigin_UpdatesOriginList(int index) { var value = "1"; _list.AddOrigin(value); _list.UpdateOrigin(value, index); _mockList.Received()[index] = value; }This change would test the
UpdateOriginmethod more thoroughly across different index values.
1-70: Overall: Well-structured and comprehensive test suiteThis test file provides good coverage for the
ObservableResolverListclass, testing all major operations (Add, Update, Remove, Clear) effectively. The use of NSubstitute for mocking is consistent and appropriate throughout the tests.To further improve the test suite, consider adding the following tests:
- Test the behavior when adding multiple items to ensure the list maintains the correct order.
- Test edge cases, such as updating or removing items at the boundaries of the list (first and last elements).
- Test the conversion functions to ensure they're working correctly, especially with edge cases (e.g., very large numbers, negative numbers).
- Test the behavior when the underlying list is initially non-empty.
Example of a test for adding multiple items:
[Test] public void AddOrigin_MultipleTimes_MaintainsCorrectOrder() { var values = new[] { "1", "2", "3" }; foreach (var value in values) { _list.AddOrigin(value); } for (int i = 0; i < values.Length; i++) { _mockList.Received().Add(values[i]); } }These additional tests would provide more comprehensive coverage and help ensure the robustness of the
ObservableResolverListimplementation.Tests/Editor/ObservableResolverDictionaryTest.cs (2)
23-38: Consider adding a comment to explain conversion logicThe
Initmethod does a great job setting up the test environment. However, the lambda expressions used in theObservableResolverDictionaryconstructor (lines 27-30) are somewhat complex. Consider adding a comment to explain the conversion logic for better readability and maintainability.
40-44: Enhance test assertion for existing keyWhile the test correctly checks if
TryGetOriginValuereturns true for an existing key, it doesn't verify the actual value returned. Consider adding an assertion to check the returned value:Assert.IsTrue(_dictionary.TryGetOriginValue(_key, out var value)); +Assert.AreEqual(_value, value);This will ensure that the correct value is being returned along with the boolean result.
CHANGELOG.md (2)
15-16: Minor grammatical improvements needed.Consider the following changes to improve clarity and grammar:
- Fixed the dispose extension methods for GameObject and Object, removing pragma directives and adding null reference check in GetValid method to avoid unwanted exceptions + Fixed the dispose extension methods for GameObject and Object, removing pragma directives and adding a null reference check in the GetValid method to avoid unwanted exceptions🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: You might be missing the article “a” here.
Context: ..., removing pragma directives and adding null reference check in GetValid method to a...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~16-~16: You might be missing the article “the” here.
Context: ...ives and adding null reference check in GetValid method to avoid unwanted exceptions...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
34-40: Improve formatting and grammar for version 0.2.0 changelog entry.Consider the following changes to improve clarity, grammar, and formatting:
- Added new *ObservableResolverList*, *ObservableResolverDictionary* & *ObservableResolverField* to allow to create observable types without referencing the collection directly - Added Unit tests to all data types in the project **Changed**: - Removed *ObservableIdList* because it's behaviour was too confusing and the same result can be obtained with *ObservableList* or *ObservableDictionary* - Removed all Pair Data and moved them to new *Pair<Key,Value>* serialized type that can now be serializable on Unity 2020.1 - Moved all Vector2, Vector3 & Vector4 extensions to the ValueData file + ### Added + - New *ObservableResolverList*, *ObservableResolverDictionary*, and *ObservableResolverField* to allow creating observable types without referencing the collection directly + - Unit tests for all data types in the project + + ### Changed + - Removed *ObservableIdList* due to confusing behavior (same functionality can be achieved with *ObservableList* or *ObservableDictionary*) + - Replaced all Pair Data with new serializable *Pair<Key,Value>* type, compatible with Unity 2020.1 + - Relocated all Vector2, Vector3, and Vector4 extensions to the ValueData fileThese changes improve the structure, grammar, and clarity of the changelog entry.
🧰 Tools
🪛 LanguageTool
[grammar] ~34-~34: Did you mean “creating”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...y* & ObservableResolverField to allow to create observable types without referencing th...(ALLOW_TO)
[grammar] ~38-~38: Did you mean to use the possessive pronoun “its”?
Context: ...:
- Removed ObservableIdList because it's behaviour was too confusing and the sam...
(IT_S_ITS)
[uncategorized] ~38-~38: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...because it's behaviour was too confusing and the same result can be obtained with *O...(COMMA_COMPOUND_SENTENCE_2)
Tests/Editor/ObservableListTest.cs (5)
33-34: Proper initialization of mock list and observable listThe initialization of
_mockListand_listin theInitmethod is correct and consistent with the earlier field changes. This setup allows for flexible mocking of list behavior.Consider adding a comment explaining the purpose of using a mock list, e.g.:
// Use a mock list to allow precise control over list behavior in tests _mockList = Substitute.For<IList<int>>();
38-42: Improved test method name and assertionThe renaming of the test method to
AddValue_AddsValueToListis a good improvement, as it clearly describes the behavior being tested. The updated assertion correctly checks the value in the new_listinstance.Consider adding an additional assertion to verify that the
Addmethod was called on the mock list:_mockList.Received(1).Add(_previousValue);This would ensure that the
ObservableListis correctly delegating theAddoperation to the underlying list.
46-57: Improved test method name and assertionsThe renaming of the test method to
SetValue_UpdatesValueis a good improvement, as it clearly describes the behavior being tested. The updated assertions correctly check the values in the new_listinstance for bothAddand indexer set operations.Consider adding assertions to verify that the underlying mock list is being updated:
_mockList.Received(1).Add(valueCheck1); _mockList.Received(1)[_index] = valueCheck2;This would ensure that the
ObservableListis correctly delegating operations to the underlying list.
63-75: Improved test precision for Observe methodThe updates to this test method improve its precision by using the new
_listinstance and verifying specific method calls on the mock caller. This approach provides a more accurate test of theObservemethod's behavior and the subsequent list operations.Consider adding assertions to verify that the underlying mock list is being updated:
_mockList.Received(1).Add(_previousValue); _mockList.Received(1)[_index] = _newValue; _mockList.Received(1).RemoveAt(_index);This would ensure that the
ObservableListis correctly delegating operations to the underlying list.
117-123: Simplified and consistent update for StopObserveCheck testThe changes to use
_listand simplify the test structure are good improvements. The test effectively verifies the behavior of theStopObservingmethod.Consider adding assertions to verify that the underlying mock list is being updated:
_mockList.Received(1).Add(_previousValue); _mockList.Received(1)[_index] = _previousValue; _mockList.Received(1).RemoveAt(_index);This would ensure that the
ObservableListis correctly delegating operations to the underlying list, even when observations are stopped.Tests/Editor/ObservableDictionaryTest.cs (3)
173-176: Confirm exception type for better specificityIn the test
InvokeUpdate_MissingKey_ThrowsException, you're asserting that aKeyNotFoundExceptionis thrown. Ensure that this is the most appropriate exception type for this scenario. If a more specific exception could be used, it might provide clearer insight into the error condition.
79-84: Verify indexer set operation with existing keyIn the test
Indexer_SetsValue_WhenKeyExists, adding an assertion before changing the value would confirm the initial state and enhance the test's clarity.Apply this diff to include the assertion:
_dictionary.Add(1, 100); +Assert.AreEqual(100, _dictionary[1]); _dictionary[1] = 200; Assert.AreEqual(200, _dictionary[1]);
34-34: Consider simplifying dictionary initializationSince
_mockDictionaryis a standardDictionary<int, int>, you might initialize_dictionarywithout explicitly passing it, unless you are testing functionality that depends on the underlying dictionary.If the underlying dictionary is crucial for certain tests, consider renaming
_mockDictionaryto_baseDictionaryor_innerDictionaryfor clarity.Runtime/ObservableList.cs (2)
5-5: Remove unused using directiveSystem.ReflectionIt appears that the
System.Reflectionnamespace is not used anywhere in this file. Removing unnecessary using directives can help keep the code clean and improve readability.
169-169: Potential issue with list casting in constructorIn the
ObservableList<T>constructor, the list is assigned using:List = list as List<T> ?? list.ToList();If
listis not aList<T>, this creates a new list viaToList(), which may not reflect changes to the originalIList<T>passed in. If the intention is to haveListalways reflect changes to the original list, consider directly using the providedIList<T>or copying it based on requirements.Runtime/ObservableDictionary.cs (3)
148-149: Correct grammatical error in documentation commentThe comment has an incorrect apostrophe in "Add's". It should be "Adds to the origin dictionary".
Apply this diff to fix the comment:
/// <remarks> - /// Add's to the origin dictionary + /// Adds to the origin dictionary /// </remarks>
154-155: Correct grammatical error in documentation commentThe comment incorrectly uses an apostrophe in "Remove's". It should be "Removes from the origin dictionary".
Apply this diff to fix the comment:
/// <remarks> - /// Remove's to the origin dictionary + /// Removes from the origin dictionary /// </remarks>
160-161: Correct grammatical error in documentation commentThe comment incorrectly uses an apostrophe in "Clear's". It should be "Clears the origin dictionary".
Apply this diff to fix the comment:
/// <remarks> - /// Clear's to the origin dictionary + /// Clears the origin dictionary /// </remarks>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- CHANGELOG.md (1 hunks)
- Runtime/ObservableDictionary.cs (8 hunks)
- Runtime/ObservableList.cs (8 hunks)
- Tests/Editor/OberservableResolverListTest.cs (1 hunks)
- Tests/Editor/OberservableResolverListTest.cs.meta (1 hunks)
- Tests/Editor/ObservableDictionaryTest.cs (2 hunks)
- Tests/Editor/ObservableListTest.cs (1 hunks)
- Tests/Editor/ObservableResolverDictionaryTest.cs (1 hunks)
- Tests/Editor/ObservableResolverDictionaryTest.cs.meta (1 hunks)
- Tests/Editor/floatPTests.cs (2 hunks)
- package.json (1 hunks)
✅ Files skipped from review due to trivial changes (3)
- Tests/Editor/OberservableResolverListTest.cs.meta
- Tests/Editor/ObservableResolverDictionaryTest.cs.meta
- package.json
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~16-~16: You might be missing the article “a” here.
Context: ..., removing pragma directives and adding null reference check in GetValid method to a...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~16-~16: You might be missing the article “the” here.
Context: ...ives and adding null reference check in GetValid method to avoid unwanted exceptions...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~34-~34: Did you mean “creating”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...y* & ObservableResolverField to allow to create observable types without referencing th...(ALLOW_TO)
[grammar] ~38-~38: Did you mean to use the possessive pronoun “its”?
Context: ...:
- Removed ObservableIdList because it's behaviour was too confusing and the sam...
(IT_S_ITS)
[uncategorized] ~38-~38: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...because it's behaviour was too confusing and the same result can be obtained with *O...(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (23)
Tests/Editor/OberservableResolverListTest.cs (3)
1-15: LGTM: Proper setup for unit testsThe namespace, imports, and test fixture setup are correctly implemented. The use of NUnit and NSubstitute is appropriate for unit testing in this context.
30-38: LGTM: Effective test for AddOrigin methodThis test correctly verifies that the
AddOriginmethod ofObservableResolverListcalls theAddmethod on the underlying list with the correct value.
62-69: LGTM: Effective test for ClearOrigin methodThis test correctly verifies that the
ClearOriginmethod ofObservableResolverListcalls theClearmethod on the underlying list.Tests/Editor/ObservableResolverDictionaryTest.cs (1)
1-22: LGTM: Well-structured test class setupThe file structure and class setup follow good practices for unit testing. The use of appropriate imports, test fixture attribute, and private fields for test data and mocks is commendable.
Tests/Editor/floatPTests.cs (5)
14-15: Improved type consistency in assertionsThe changes in the
Representationmethod enhance type safety and consistency. By casting float literals tofloatPin assertions (e.g.,(floatP)0finstead of0f), we ensure that comparisons are made between values of the same type. This modification not only makes the tests more accurate but also aligns with best practices for strongly-typed languages.Also applies to: 17-23
45-46: Consistent type casting in arithmetic operationsThe modifications in the
Additionmethod follow the same pattern as in theRepresentationmethod. Casting the results of arithmetic operations tofloatP(e.g.,(floatP)2f,(floatP)0f) ensures type consistency and makes the tests more robust. This change maintains the improved type safety throughout the test class.
52-55: Proper type casting for infinity operationsThe changes in the
Multiplicationmethod forPositiveInfinityoperations are consistent with the previous modifications. By casting the results tofloatP, we ensure that these edge cases are tested with the correct type. This is particularly important for operations involving infinity, as it verifies thatfloatPcorrectly handles these special cases in the same manner as the standardfloattype.
57-60: Symmetrical testing for negative infinity operationsThe modifications for
NegativeInfinityoperations mirror those made forPositiveInfinity. This symmetrical approach to testing both positive and negative infinity operations is crucial for ensuring thatfloatPhandles all edge cases correctly. The consistent casting tofloatPmaintains type safety and provides a comprehensive test suite for infinity-related arithmetic.
62-62: Consistent type casting for all operationsThe final modification in the
Multiplicationmethod, casting the result offloatP.One * floatP.Oneto(floatP)1f, maintains consistency with all previous changes. This attention to detail ensures that even simple operations are tested with proper type consistency.Overall, these modifications throughout the
floatPTestsclass significantly improve the robustness and reliability of the test suite. By consistently casting float literals and operation results tofloatP, we ensure that all comparisons and assertions are performed using the correct type. This approach not only enhances type safety but also provides a more accurate validation of thefloatPtype's behavior across various scenarios, including edge cases and basic arithmetic operations.CHANGELOG.md (4)
7-10: LGTM: Version 0.6.0 changes are well-documented.The changelog entry for version 0.6.0 accurately reflects the improvements mentioned in the PR objectives. The description is clear and concise, providing valuable information about the enhancements to ObservableResolverList and ObservableResolverDictionary.
18-20: LGTM: Version 0.5.0 changes are well-documented.The changelog entry for version 0.5.0 accurately describes the addition of the floatP type, including its purpose and features. The description is clear and informative, providing valuable context for the new functionality.
22-25: LGTM: Version 0.4.0 changes are well-documented.The changelog entries for version 0.4.0 accurately describe the additions of utility methods, extensions, and the SerializableType struct. The descriptions are clear and concise, providing a good overview of the new functionality.
27-30: LGTM: Version 0.3.0 changes are well-documented.The changelog entries for version 0.3.0 accurately describe the additions of field update observation support and the UnitySerializedDictionary class. The descriptions are clear and concise, providing a good overview of the new functionality.
Tests/Editor/ObservableListTest.cs (6)
25-26: Improved test setup with mock listThe replacement of separate observable list instances with a single
ObservableList<int>backed by a mockedIList<int>is a good improvement. This change simplifies the test setup and allows for better control and verification of list operations.
83-86: Improved test precision for InvokeObserve methodThe updates to this test method improve its precision by using the new
_listinstance and verifying specific method calls on the mock caller. This approach provides a more accurate test of theInvokeObservemethod's behavior.
94-101: Improved test precision for InvokeUpdate methodThe updates to this test method improve its precision by using the new
_listinstance and verifying specific method calls on the mock caller. This approach provides a more accurate test of theInvokeUpdatemethod's behavior.
109-111: Consistent update for InvokeUpdate_NotObserving_DoesNothing testThe change to use
_listinstead of_observableListis consistent with the earlier refactoring. The test correctly verifies thatInvokeUpdatedoes nothing when not observing.
131-146: Simplified and consistent updates for StopObservingAll testsThe changes to use
_listand simplify the test structure in bothStopObservingAllCheckandStopObservingAll_MultipleCalls_Checkare good improvements. These tests effectively verify the behavior of theStopObservingAllmethod in different scenarios.
154-171: Consistent updates for remaining StopObservingAll testsThe changes to use
_listin theStopObservingAll_Everything_CheckandStopObservingAll_NotObserving_DoesNothingtests are consistent with the earlier refactoring. These tests effectively verify the behavior of theStopObservingAllmethod when called with no arguments and when not observing.Tests/Editor/ObservableDictionaryTest.cs (3)
199-203: Test method correctly named and implementedThe test method
InvokeUpdate_NotObserving_DoesNothingfollows the naming convention and accurately tests the expected behavior when no observers are registered.
276-281: Test method correctly named and implementedThe test method
StopObservingAll_NotObserving_DoesNothingadheres to the naming convention and accurately tests the behavior when no observers are registered.
147-148: Clarify observer registration in testsIn the
ObserveChecktest, both key-specific and global observers are registered. Ensure the test separately verifies the notifications received by each observer to confirm that both are functioning correctly.You can enhance the test by distinguishing between calls received by key-specific and global observers.
Runtime/ObservableList.cs (1)
250-252: Optimize nested loop inClearmethodIn the
Clearmethod ofObservableList<T>, there's a nested loop that could be optimized:for (var i = 0; i < _updateActions.Count; i++) { for (var j = 0; j < list.Count; j++) { _updateActions[i](j, list[j], default, ObservableUpdateType.Removed); } }Consider combining the loops or reducing the number of iterations to improve performance, especially for large lists.
[performance]
| [Test] | ||
| public void AddOrigin_AddsValueToOriginDictionary() | ||
| { | ||
| _dictionary.AddOrigin(_key, _value); | ||
|
|
||
| _mockDictionary.Received().Add(_key, _value); | ||
| } | ||
|
|
||
| [Test] | ||
| public void UpdateOrigin_UpdatesValueInOriginDictionary() | ||
| { | ||
| _dictionary.AddOrigin(_key, _value); | ||
| _dictionary.UpdateOrigin(_key, _value); | ||
|
|
||
| _mockDictionary.Received()[_key] = _value; | ||
| } | ||
|
|
||
| [Test] | ||
| public void RemoveOrigin_RemovesValueFromOriginDictionary() | ||
| { | ||
| _dictionary.AddOrigin(_key, _value); | ||
|
|
||
| Assert.IsTrue(_dictionary.RemoveOrigin(_key)); | ||
| _mockDictionary.Received().Remove(_key); | ||
| } | ||
|
|
||
| [Test] | ||
| public void ClearOrigin_ClearsOriginDictionary() | ||
| { | ||
| _dictionary.ClearOrigin(); | ||
|
|
||
| _mockDictionary.Received().Clear(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage for dictionary operations
The tests for AddOrigin, UpdateOrigin, RemoveOrigin, and ClearOrigin cover basic functionality, but could be more comprehensive:
-
For
AddOrigin, consider verifying that the value can be retrieved after adding:_dictionary.AddOrigin(_key, _value); Assert.IsTrue(_dictionary.TryGetOriginValue(_key, out var retrievedValue)); Assert.AreEqual(_value, retrievedValue);
-
For
UpdateOrigin, verify that the value was actually updated:_dictionary.UpdateOrigin(_key, "new_value"); Assert.IsTrue(_dictionary.TryGetOriginValue(_key, out var updatedValue)); Assert.AreEqual("new_value", updatedValue);
-
For
RemoveOrigin, check that the key no longer exists after removal:Assert.IsTrue(_dictionary.RemoveOrigin(_key)); Assert.IsFalse(_dictionary.TryGetOriginValue(_key, out _));
-
For
ClearOrigin, verify that the dictionary is empty after clearing:_dictionary.ClearOrigin(); Assert.AreEqual(0, _dictionary.Count);
These additions will provide more robust verification of the ObservableResolverDictionary behavior.
| using System; | ||
| using System.Collections; | ||
| using System.Collections.Generic; | ||
| using System.Collections.ObjectModel; | ||
| using System.Reflection; | ||
| using GameLovers; | ||
| using NSubstitute; | ||
| using NUnit.Framework; | ||
| using UnityEngine; | ||
|
|
||
| // ReSharper disable once CheckNamespace | ||
|
|
||
| namespace GameLoversEditor.DataExtensions.Tests | ||
| { | ||
| [TestFixture] | ||
| public class ObservableResolverDictionaryTest | ||
| { | ||
| private int _key = 0; | ||
| private string _value = "1"; | ||
| private ObservableResolverDictionary<int, int, int, string> _dictionary; | ||
| private IDictionary<int, string> _mockDictionary; | ||
|
|
||
| [SetUp] | ||
| public void Init() | ||
| { | ||
| _mockDictionary = Substitute.For<IDictionary<int, string>>(); | ||
| _dictionary = new ObservableResolverDictionary<int, int, int, string>( | ||
| _mockDictionary, | ||
| origin => new KeyValuePair<int, int>(origin.Key, int.Parse(origin.Value)), | ||
| (key, value) => new KeyValuePair<int, string>(key, key.ToString())); | ||
|
|
||
| _mockDictionary[_key].Returns(_value); | ||
| _mockDictionary.TryGetValue(_key, out _).Returns(callInfo => | ||
| { | ||
| callInfo[1] = _mockDictionary[_key]; | ||
| return true; | ||
| }); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TryGetOriginValue_KeyExists_ReturnsTrueAndOutValue() | ||
| { | ||
| Assert.IsTrue(_dictionary.TryGetOriginValue(_key, out var value)); | ||
| } | ||
|
|
||
| [Test] | ||
| public void TryGetOriginValue_KeyDoesNotExist_ReturnsFalseAndOutDefault() | ||
| { | ||
| var result = _dictionary.TryGetOriginValue(999, out var value); | ||
|
|
||
| Assert.IsFalse(result); | ||
| Assert.IsNull(value); | ||
| } | ||
|
|
||
| [Test] | ||
| public void AddOrigin_AddsValueToOriginDictionary() | ||
| { | ||
| _dictionary.AddOrigin(_key, _value); | ||
|
|
||
| _mockDictionary.Received().Add(_key, _value); | ||
| } | ||
|
|
||
| [Test] | ||
| public void UpdateOrigin_UpdatesValueInOriginDictionary() | ||
| { | ||
| _dictionary.AddOrigin(_key, _value); | ||
| _dictionary.UpdateOrigin(_key, _value); | ||
|
|
||
| _mockDictionary.Received()[_key] = _value; | ||
| } | ||
|
|
||
| [Test] | ||
| public void RemoveOrigin_RemovesValueFromOriginDictionary() | ||
| { | ||
| _dictionary.AddOrigin(_key, _value); | ||
|
|
||
| Assert.IsTrue(_dictionary.RemoveOrigin(_key)); | ||
| _mockDictionary.Received().Remove(_key); | ||
| } | ||
|
|
||
| [Test] | ||
| public void ClearOrigin_ClearsOriginDictionary() | ||
| { | ||
| _dictionary.ClearOrigin(); | ||
|
|
||
| _mockDictionary.Received().Clear(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider additional test scenarios for comprehensive coverage
The current test suite provides good coverage of basic operations for ObservableResolverDictionary. To further enhance the robustness of your tests, consider adding the following scenarios:
- Test the behavior when adding a duplicate key.
- Verify that the
Countproperty is updated correctly after add, remove, and clear operations. - Test the
ContainsKeymethod for both existing and non-existing keys. - Verify the behavior of the
KeysandValuesproperties. - Test the
GetEnumeratormethod to ensure proper iteration over the dictionary. - Add tests for any custom events or notifications that
ObservableResolverDictionarymight implement.
These additional tests will help ensure that all aspects of the ObservableResolverDictionary class are thoroughly verified.
|
|
||
| - Improved the *ObservableResolverList* and *ObservableResolverDictionary* data types to properly resolve lists and dictionaries with different data types from the original collection. | ||
|
|
||
| ## [0.5.1] - 2023-09-04 |
There was a problem hiding this comment.
Inconsistent version date.
The date for version 0.5.1 (2023-09-04) is later than the date for version 0.6.0 (2023-08-05). This appears to be an inconsistency in the changelog. Please verify and adjust the dates to maintain chronological order.
| public void ValueSetCheck() | ||
| { | ||
| const int valueCheck1 = 5; | ||
| const int valueCheck2 = 6; | ||
|
|
||
| _mockDictionary.Add(_key, valueCheck1); | ||
| _dictionary[_key] = valueCheck2; | ||
|
|
||
| Assert.AreNotEqual(valueCheck1, _mockDictionary[_key]); | ||
| Assert.AreEqual(valueCheck2, _dictionary[_key]); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent test method naming conventions
The test method ValueSetCheck does not follow the naming convention used in other test methods. For clarity and consistency, consider renaming it to ValueSet_UpdatesUnderlyingDictionary or ValueSet_UpdatesValueSuccessfully to match the Method_Scenario_ExpectedResult pattern.
| public void ObserveCheck() | ||
| { | ||
| var startValue = 0; | ||
| var newValue = 1; | ||
|
|
||
| _dictionary.Observe(_key, _caller.Call); | ||
| _dictionary.Observe(_caller.Call); | ||
|
|
||
| _dictionary.Add(_key, startValue); | ||
|
|
||
| _dictionary[_key] = newValue; | ||
|
|
||
| _dictionary.Remove(_key); | ||
|
|
||
| _caller.Received().Call(_key, 0, startValue, ObservableUpdateType.Added); | ||
| _caller.Received().Call(_key, startValue, newValue, ObservableUpdateType.Updated); | ||
| _caller.Received().Call(_key, newValue, 0, ObservableUpdateType.Removed); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent test method naming conventions
The test method ObserveCheck differs from the naming convention used elsewhere. To enhance readability, consider renaming it to Observe_NotifiesObservers_OnChanges or similar, aligning with the Method_Scenario_ExpectedResult format.
| public bool Remove(T data) | ||
| { | ||
| var idx = List.IndexOf(data); | ||
|
|
||
| if (idx >= 0) | ||
| { | ||
| RemoveAt(idx); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider marking the Remove method as virtual for consistency
In the ObservableList<T> class, the Add, RemoveAt, and Clear methods are marked as virtual, allowing derived classes to override them. However, the Remove method is not marked as virtual. For consistency and extensibility, consider marking Remove as virtual.
Apply this diff:
-public bool Remove(T data)
+public virtual bool Remove(T data)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public bool Remove(T data) | |
| { | |
| var idx = List.IndexOf(data); | |
| if (idx >= 0) | |
| { | |
| RemoveAt(idx); | |
| return true; | |
| } | |
| return false; | |
| public virtual bool Remove(T data) | |
| { | |
| var idx = List.IndexOf(data); | |
| if (idx >= 0) | |
| { | |
| RemoveAt(idx); | |
| return true; | |
| } | |
| return false; |
| public override void Clear() | ||
| { | ||
| _dictionary.Clear(); | ||
| base.Clear(); ; |
There was a problem hiding this comment.
Remove duplicate semicolon to fix syntax error
There's a duplicate semicolon on this line:
base.Clear(); ;This will cause a syntax error. Remove the extra semicolon.
Apply this diff to fix the syntax error:
- base.Clear(); ;
+ base.Clear();| public override bool Remove(TKey key) | ||
| { | ||
| var pair = _toOrignResolver(key, Dictionary[key]); | ||
|
|
||
| _dictionary.Remove(pair.Key); | ||
|
|
||
| return base.Remove(key); | ||
| } |
There was a problem hiding this comment.
Ensure key exists before accessing Dictionary[key] in Remove method
In the Remove method of ObservableResolverDictionary, you access Dictionary[key] without checking if the key exists. This can lead to a KeyNotFoundException if the key is not present in the dictionary.
Consider verifying that the key exists before accessing it:
public override bool Remove(TKey key)
{
+ if (!Dictionary.TryGetValue(key, out var value))
+ {
+ return false;
+ }
- var pair = _toOrignResolver(key, Dictionary[key]);
+ var pair = _toOrignResolver(key, value);
_dictionary.Remove(pair.Key);
return base.Remove(key);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public override bool Remove(TKey key) | |
| { | |
| var pair = _toOrignResolver(key, Dictionary[key]); | |
| _dictionary.Remove(pair.Key); | |
| return base.Remove(key); | |
| } | |
| public override bool Remove(TKey key) | |
| { | |
| if (!Dictionary.TryGetValue(key, out var value)) | |
| { | |
| return false; | |
| } | |
| var pair = _toOrignResolver(key, value); | |
| _dictionary.Remove(pair.Key); | |
| return base.Remove(key); | |
| } |
| public void UpdateOrigin(TKeyOrigin key, TValueOrigin value) | ||
| { | ||
| var convertPair = _fromOrignResolver(new KeyValuePair<TKeyOrigin, TValueOrigin>(key, value)); | ||
|
|
||
| _dictionary[key] = value; | ||
| this[convertPair.Key] = convertPair.Value; | ||
| } |
There was a problem hiding this comment.
Check for existence of origin key before updating in UpdateOrigin method
In the UpdateOrigin method, you update _dictionary[key] without verifying if the key exists. Assigning to a non-existent key will add a new entry, which might not be intended.
Consider adding a check to ensure the key exists before updating:
public void UpdateOrigin(TKeyOrigin key, TValueOrigin value)
{
var convertPair = _fromOrignResolver(new KeyValuePair<TKeyOrigin, TValueOrigin>(key, value));
+ if (!_dictionary.ContainsKey(key))
+ {
+ throw new KeyNotFoundException($"The key '{key}' does not exist in the origin dictionary.");
+ }
_dictionary[key] = value;
this[convertPair.Key] = convertPair.Value;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void UpdateOrigin(TKeyOrigin key, TValueOrigin value) | |
| { | |
| var convertPair = _fromOrignResolver(new KeyValuePair<TKeyOrigin, TValueOrigin>(key, value)); | |
| _dictionary[key] = value; | |
| this[convertPair.Key] = convertPair.Value; | |
| } | |
| public void UpdateOrigin(TKeyOrigin key, TValueOrigin value) | |
| { | |
| var convertPair = _fromOrignResolver(new KeyValuePair<TKeyOrigin, TValueOrigin>(key, value)); | |
| if (!_dictionary.ContainsKey(key)) | |
| { | |
| throw new KeyNotFoundException($"The key '{key}' does not exist in the origin dictionary."); | |
| } | |
| _dictionary[key] = value; | |
| this[convertPair.Key] = convertPair.Value; | |
| } |
| private readonly IDictionary<TKeyOrigin, TValueOrigin> _dictionary; | ||
| private readonly Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> _toOrignResolver; | ||
| private readonly Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> _fromOrignResolver; | ||
|
|
||
| protected override IDictionary<TKey, TValue> Dictionary => _dictionaryResolver(); | ||
| /// <inheritdoc /> | ||
| public ReadOnlyDictionary<TKeyOrigin, TValueOrigin> OriginDictionary => new ReadOnlyDictionary<TKeyOrigin, TValueOrigin>(_dictionary); | ||
|
|
||
| public ObservableResolverDictionary(Func<IDictionary<TKey, TValue>> dictionaryResolver) | ||
| public ObservableResolverDictionary(IDictionary<TKeyOrigin, TValueOrigin> dictionary, | ||
| Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> fromOrignResolver, | ||
| Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> toOrignResolver) | ||
| : base(new Dictionary<TKey, TValue>(dictionary.Count)) | ||
| { | ||
| _dictionary = dictionary; | ||
| _toOrignResolver = toOrignResolver; | ||
| _fromOrignResolver = fromOrignResolver; | ||
|
|
There was a problem hiding this comment.
Correct spelling of field names _toOrignResolver and _fromOrignResolver to _toOriginResolver and _fromOriginResolver
The field names _toOrignResolver and _fromOrignResolver contain a misspelling of the word "Origin".
Rename the fields to correct the spelling and update all references:
- private readonly Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> _toOrignResolver;
- private readonly Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> _fromOrignResolver;
+ private readonly Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> _toOriginResolver;
+ private readonly Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> _fromOriginResolver;Ensure that you also update these field names throughout the class.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private readonly IDictionary<TKeyOrigin, TValueOrigin> _dictionary; | |
| private readonly Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> _toOrignResolver; | |
| private readonly Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> _fromOrignResolver; | |
| protected override IDictionary<TKey, TValue> Dictionary => _dictionaryResolver(); | |
| /// <inheritdoc /> | |
| public ReadOnlyDictionary<TKeyOrigin, TValueOrigin> OriginDictionary => new ReadOnlyDictionary<TKeyOrigin, TValueOrigin>(_dictionary); | |
| public ObservableResolverDictionary(Func<IDictionary<TKey, TValue>> dictionaryResolver) | |
| public ObservableResolverDictionary(IDictionary<TKeyOrigin, TValueOrigin> dictionary, | |
| Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> fromOrignResolver, | |
| Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> toOrignResolver) | |
| : base(new Dictionary<TKey, TValue>(dictionary.Count)) | |
| { | |
| _dictionary = dictionary; | |
| _toOrignResolver = toOrignResolver; | |
| _fromOrignResolver = fromOrignResolver; | |
| private readonly IDictionary<TKeyOrigin, TValueOrigin> _dictionary; | |
| private readonly Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> _toOriginResolver; | |
| private readonly Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> _fromOriginResolver; | |
| /// <inheritdoc /> | |
| public ReadOnlyDictionary<TKeyOrigin, TValueOrigin> OriginDictionary => new ReadOnlyDictionary<TKeyOrigin, TValueOrigin>(_dictionary); | |
| public ObservableResolverDictionary(IDictionary<TKeyOrigin, TValueOrigin> dictionary, | |
| Func<KeyValuePair<TKeyOrigin, TValueOrigin>, KeyValuePair<TKey, TValue>> fromOrignResolver, | |
| Func<TKey, TValue, KeyValuePair<TKeyOrigin, TValueOrigin>> toOrignResolver) | |
| : base(new Dictionary<TKey, TValue>(dictionary.Count)) | |
| { | |
| _dictionary = dictionary; | |
| _toOriginResolver = toOrignResolver; | |
| _fromOriginResolver = fromOrignResolver; |
-Improved the ObservableResolverList and ObservableResolverDictionary data types to properly resolve lists and dictionaries with different data types from the original collection.
-Fixed Unit tests for floatPTests that were failing.
Summary by CodeRabbit
Release Notes:
floatPTestsand adjusted assertions forfloatPvalues.ObservableResolverListandObservableResolverDictionary.ObservableResolverListandObservableResolverDictionaryto ensure proper functionality and coverage.Summary by CodeRabbit
Release Notes
New Features
StructPairandfloatPdata types for improved memory usage and precision.ObservableResolverListandObservableResolverDictionaryclasses for enhanced list and dictionary functionalities.Improvements
ObservableListandObservableDictionaryto support complex interactions and observability.ObservableResolverList,ObservableResolverDictionary, andfloatP.Bug Fixes
Documentation