-
Notifications
You must be signed in to change notification settings - Fork 192
pop, drop & get with basic range feature for stringlist #514
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
Conversation
delete
function vs remove
function?
delete
functions returns the deleted element.
remove
function does same job as to delete
function but doesn't return the deleted element.
Any suggestions on better naming convention? Should I rather call delete
as pop
and think of a better name for remove
?
epagone
commented
Sep 7, 2021
This is highly subjective but FWIW, I would prefer pop
for the functions that return the deleted element, simply because I imagine that if I "pop" something out I still have it with me.
For the function that does not return the deleted element I am fine with both remove
or delete
, with a slight preference for delete
.
Did you have any time to see what are the choices of other languages for similar functions?
Python uses pop
(returns deleted item) and remove
(no return)
Java doesn't have any function equivalent of pop
(or atleast I couldn't find one), it has remove
function (no return).
pop
function is used whenever deleted item is returned, otherwise remove
and delete
functions both are used by programming languages. Though, I could find only one occurrence where delete
was used to remove elements.
Also, some languages uses pop()
(with no arguments) as a way to allow a user to make a list
behave like a stack
or a queue
.
I think pop
is a reasonable name to a procedure to remove and return the removed element. For only deleting elements, delete
, remove
or drop
might be suitable. The names delete
and destroy
could be associated with a deconstructor of the entire list.
epagone
commented
Sep 10, 2021
Based on my experience with past discussions, we are trying to provide a familiar interface to the user of stdlib
following well-established conventions from other languages (like python, for example). @arjenmarkus suggests more "Fortranic" names (that I like too) but they might be inconsistent with other parts of stdlib
.
Based on the other suggestions, my preference is
pop
: removes and returns the removed elementdrop
: removes an element without returning itdestroy
: de-constructs the list and frees the memory
I do not have strong opinions on this, though.
I agree with destroy
(please note that we have a function named clear
in stdlib_stringlist
which resets the list and NOT destroys it). Checkout this example from python to understand the difference:
I added a basic range feature to pop
and drop
which will remove all strings at indexes in the interval [first
, last
].
Out of bounds indexes in this interval will be ignored. There is no concept of stride
currently in this range feature.
We can discuss the behaviour of non-basic range feature for pop
and drop
.
I think atleast a basic range feature should be added to get
function as well, it will also improve the way pop_engine
function is written.
Thank you.
I personally don't like that out of bounds indexes are ignored: I'd like to be notified (at least with a warning, if not by an error) in these cases. Based on my experience, out of bounds indexes are the consequence of upstream bugs that would be difficult to identify, if ignored. However, I believe that a choice in this sense might have been already made during the initial implementation of stringlist
for other methods.
I think atleast a basic range feature should be added to
get
function as well, it will also improve the waypop_engine
function is written.
Good point, I agree.
4391972
to
a02c5ed
Compare
epagone
commented
Sep 17, 2021
I am under the impression that adding the get
function (although, as I said, it's a good idea) is out of scope for this PR. Am I right?
I am under the impression that adding the
get
function (although, as I said, it's a good idea) is out of scope for this PR. Am I right?
Yeah, that's right. Let me rename PR.
Co-authored-by: Emanuele Pagone <epagone.dev@gmail.com>
Co-authored-by: Emanuele Pagone <epagone.dev@gmail.com>
77bca62
to
2b3a5ef
Compare
This PR is blocked by: #552
get_engine func takes an integer stride_vector which decides the number of strides to take between indexes. get_range_idx func takes stride of type stringlist_index_type: fidx(+3) means takes +3 as stride (jump 3 indexes to right) fidx(-3) means takes -3 as stride (jump 3 indexes to left) bidx(+3) means takes -3 as stride (jump 3 indexes to left) bidx(-3) means takes +3 as stride (jump 3 indexes to right)
get_impl comes in the middle of get_engine and get_idx routines This allows to get i-th element inside the module using integer indexes
Uh oh!
There was an error while loading. Please reload this page.
Delete function to
stdlib_stringlist
Tasks:
impure
) to stringlistdelete
topop
and also addeddrop
functionpop
,drop
andget
move
ofstdlib_string_type
pure and some other subroutines which should have been pure