stdlib icon indicating copy to clipboard operation
stdlib copied to clipboard

pop, drop & get with basic range feature for stringlist

Open aman-godara opened this issue 3 years ago • 12 comments

Delete function to stdlib_stringlist

Tasks:

  • [x] Add delete function (impure) to stringlist
  • [x] renamed delete to pop and also added drop function
  • [x] corrected error in documentation #516
  • [x] added range pop, drop and get
  • [x] made subroutine move of stdlib_string_type pure and some other subroutines which should have been pure
  • [ ] Add test cases for the same
  • [ ] Documentation

aman-godara avatar Sep 06 '21 16:09 aman-godara

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?

aman-godara avatar Sep 06 '21 16:09 aman-godara

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?

epagone avatar Sep 07 '21 11:09 epagone

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.

aman-godara avatar Sep 07 '21 13:09 aman-godara

How about get_and_remove or get_remove? That makes the function completely clear, albeit via a more verbose name.

Op di 7 sep. 2021 om 15:29 schreef Aman Godara @.***>:

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.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fortran-lang/stdlib/pull/514#issuecomment-914307491, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN6YR7NQ6N7APHXMGEGK4LUAYHTBANCNFSM5DQYEJAQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

arjenmarkus avatar Sep 07 '21 14:09 arjenmarkus

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.

awvwgk avatar Sep 08 '21 20:09 awvwgk

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 element
  • drop: removes an element without returning it
  • destroy: de-constructs the list and frees the memory

I do not have strong opinions on this, though.

epagone avatar Sep 10 '21 12:09 epagone

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:

image

aman-godara avatar Sep 10 '21 17:09 aman-godara

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.

aman-godara avatar Sep 12 '21 12:09 aman-godara

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 way pop_engine function is written.

Good point, I agree.

epagone avatar Sep 13 '21 11:09 epagone

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?

epagone avatar Sep 17 '21 10:09 epagone

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.

aman-godara avatar Sep 17 '21 12:09 aman-godara

This PR is blocked by: #552

aman-godara avatar Oct 17 '21 11:10 aman-godara