-
Notifications
You must be signed in to change notification settings - Fork 2
Add API for "AsValue()" type functions #13
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
base: master
Are you sure you want to change the base?
Conversation
Deprecate older function names #closes hosannahighertech#9
|
Can you add operator overloading [] as the linked ticket says? Also I will wait for tests that should cover this (and may be the other [] part?) before merging |
mtangoo
left a comment
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.
We need to have an overloaded []
|
Got it. I'll add |
|
Take your time. |
This should not be public because this is actually copying what the node is pointing to, not just its content. It is easy to cause a circular reference if you don't know what you are doing and clients should no need to call a low-level function like this anyway.
If you request an array from strings, but the property contains numbers, then the returned vector will now be empty.
|
I have implemented everything except for So, I tried doing this, but the API winds up being really awkward because we use
IMO, that looks more confusing than What do you think? |
|
Hi @Blake-Madden thanks for great working you are doing. Let me know when you are done for full review and merging |
|
This is ready now. I've added every unit test that I can think of and the The |
|
Thanks for your time and effort. I'll definitely review and merge |
Does the same thing, but is much cleaner and easier to read.
Deprecate older function names
Improve documentation
Adds unit tests
Fix a few minor issues discovered from unit tests
Make
Add()overload that can be unsafe privateCloses #9
Closes #2
Fixes part of #10