-
-
Notifications
You must be signed in to change notification settings - Fork 378
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
Added support of collection initializer syntax. #250
Added support of collection initializer syntax. #250
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks really good! Apart from a few nitpicks, I'd recommend adding more tests, mostly to test the various possible exceptions. Also, I believe the current implementation can't parse new List<string> { { string.Empty } }
.
c1d5b11
to
f34fce8
Compare
099648b
to
0c508af
Compare
0ac0258
to
990d579
Compare
I have added scenarios within the differing unit tests to cover each of the cases mentioned. Is it preferred that they become standalone cases to better show what negative case they are trying to cover? |
Thank you @holdenmai
Yes, maybe having separate tests would be better... or try to simplify/explain better the existing ones. |
…lustrate what compilation scenario they are covering.
I have reorganized the unit tests to better show the different scenarios being covered. |
LGTM! @metoule ? There is just 3 conversations still open that I'm not sure if they are now resolved ... |
Sorry for the late review! The code looks good to me, the test cases as well, I still have a few questions, but nothing major. |
…t of assigning to a parameter in the call of the add method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!! 🚀
Implementation of collection initializer syntax support for constructor #194 Managed to somehow close the original PR on accident during rebase operation pull 246, but all comments have now been resolved.
You can now write
as well as implementations supporting multiple parameters in the Add method such as
new MyClassAdder(){{ 1, 2, 3, 4, 5},{\"6\" },7 }
and
new Dictionary<int, string>(){{1, \"1\"}, {2, \"2\"}, {3, \"3\"}, {4, \"4\"}, {5, \"5\"}}
Reference to method parameters is also supported like below
target.Eval<System.Collections.Generic.List<string>>("new List<string>(){StrProp = string.Empty}", new Parameter("StrProp", "0")) target.Eval<System.Collections.Generic.List<string>>("new List<string>(){StrValue()}", new Parameter("StrValue", new Func<string>(() => "Func")))