Add Function to retrieve all Keys in a Hash #590
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commy2
reviewed
Feb 14, 2017
addons/hashes/CfgFunctions.hpp
Outdated
| // CBA_fnc_hashGetKeys | ||
| class hashGetKeys | ||
| { | ||
| description = "Returns all the Keys in a Hash"; |
There was a problem hiding this comment.
Returns all the Keys in a Hash .
commy2
reviewed
Feb 14, 2017
addons/hashes/fnc_hashGetKeys.sqf
Outdated
| Function: CBA_fnc_hashGetKeys | ||
|
|
||
| Description: | ||
| Returns all the Keys in a Hash |
commy2
suggested changes
Feb 14, 2017
|
My imagination to make up tests is not that good. |
|
test fails, because of this line: |
|
Also put a period after "Returns all the Keys in a Hash" while you're at it. Thanks. |
|
All the pointy things! |
execVM "\x\cba\addons\hashes\test_hashes.sqf" |
commy2
reviewed
Feb 15, 2017
addons/hashes/test_hashes.sqf
Outdated
| [_hash, "123", 1] call CBA_fnc_hashSet; | ||
| [_hash, "124", 2] call CBA_fnc_hashSet; | ||
| [_hash, 125, 3] call CBA_fnc_hashSet; | ||
| _keys = [_hash] call CBA_fnc_hashSize; |
commy2
reviewed
Feb 15, 2017
addons/hashes/test_hashes.sqf
Outdated
| [_hash, "124", 2] call CBA_fnc_hashSet; | ||
| [_hash, 125, 3] call CBA_fnc_hashSet; | ||
| _keys = [_hash] call CBA_fnc_hashSize; | ||
| TEST_OP(_keys,isEqualTo,[ARR_3("123","124",125)],"hashSize"); |
|
Should this function be called |
|
I thought about that. But with getKeys it's more clear that you get something IMO. |
|
Thats what you get for copy pasting stuff. |
|
commy2
reviewed
Feb 15, 2017
addons/hashes/fnc_hashGetKeys.sqf
Outdated
| // ----------------------------------------------------------------------------- | ||
| params [["_hash", [[], []], [[]]]]; | ||
|
|
||
| (_hash select HASH_KEYS) |
There was a problem hiding this comment.
newline @ EOF
Should this array be copied?
There was a problem hiding this comment.
What?! I swear I have asked if you think it should be copied. Apparently I thought of it but never did it. My brain is smoosh. Yeah.. Copy is probably better But there may also be a use for byref
dedmen
force-pushed
the
hashGetKeys
branch
2 times, most recently
from
89ae448
to
8c708ea
Compare
|
Should finally be all fine now. Except maybe renaming it to hashKeys. Your decision. |
|
Now you put two newlines at the end. |
|
I'd prefer |
Add pointy things Add Tests
|
I'll never make a small CBA function again. It's too much hassle ^^ |
commy2
reviewed
Feb 15, 2017
addons/hashes/test_hashes.sqf
Outdated
| [_hash, "124", 2] call CBA_fnc_hashSet; | ||
| [_hash, 125, 3] call CBA_fnc_hashSet; | ||
| _keys = [_hash] call CBA_fnc_hashKeys; | ||
| TEST_OP(_keys,isEqualTo,[ARR_3("123","124",125)],"hashGetKeys"); |
commy2
reviewed
Feb 15, 2017
addons/hashes/test_hashes.sqf
Outdated
| // Empty hash keys | ||
| _hash = [] call CBA_fnc_hashCreate; | ||
| _keys = [_hash] call CBA_fnc_hashKeys; | ||
| TEST_OP(_keys,isEqualTo,[],"hashGetKeys"); |
I could've fixed it all in one go if I had push access to your branch. Maybe a secondary PR next time... |
|
Edits from maintainers are allowed :x So you should have access |
|
lol |
shukari
added a commit
to shukari/task-force-arma-3-radio
that referenced
this pull request
Apr 18, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am currently doing
params ["", "_keys"]to get the keys of a hash.But that might break if the internal structure changes.
So rather have a offical API function for that.