Conversation

ChetanKarwa

This PR is to submit the first draft of the linked list module for stdlib.
This is based upon #68 by Milan, #463 by me.

I have been working on this module as a part of my GSoC Project.
All my work was regularly updated on my personal repository that I have been sharing with everyone in my weekly report.
Link to my repository (here).
A bit detailed project report is available (here).
List of all the weekly Blogs - (here)

The module was tested in two compilers, intel oneAPI and gfortran.

@awvwgkawvwgk added topic: container(Abstract) data structures and containersreviewers neededThis requires extra eyeslabels Aug 21, 2021
@zozihazoziha mentioned this pull request Aug 26, 2021
7 tasks
@awvwgk

How should we review this best? While there is an implementation I don't find a specification, examples or tests in this PR.

@arjenmarkus

@jvdp1jvdp1 added the waiting for OPThis requires action from the OPlabel Nov 10, 2021
@awvwgk

Any update on this feature?

@arjenmarkus

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This linked list is very good 👍 and can store almost any data type, I leave a little suggestion based on my limited knowledge, maybe not quite right :)

deallocate(current_node)
this_child_list%num_nodes = this_child_list%num_nodes - 1
end do

Copy link
Contributor

@zoziha zoziha Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
nullify (this_child_list%head, this_child_list%tail)

It seems that the head and tail of the linked list need to be blanked here, otherwise an error will be reported when a new push node is pushed again.

current_node = this_node
next_node => current_node%next
do
deallocate(current_node)
Copy link
Contributor

@zoziha zoziha Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
deallocate(current_node)
call current_node%clear()
deallocate(current_node)

It seems that the destruction of the current node content should be added here.


if (index == node_index) then
! Return the pointer to item stored at specified index
return_item => current_node%item
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a routine can be abstracted here, allowing the user to customize the way to get the contents of the linked list?

...
generic :: get => get_node_at_index, &
                  get_node_at_index_user
procedure, private :: get_node_at_index, &
                      get_node_at_index_user
...
abstract interface
        subroutine get_value(this_item, return_item)
            class(*), intent(in) :: this_item
            class(*), intent(out) :: return_item
        end subroutine get_value
end interface
...
subroutine get_node_at_index_user(this_list, node_index, func, return_item)
        class(child_list), intent(inout) :: this_list
        integer, intent(in) :: node_index
        procedure(get_value) :: func
        class(*), intent(out) :: return_item
        type(node), pointer :: curr_node
        integer index

        curr_node => this_list%head
        index = 1
        do while (associated(curr_node))
            if (index == node_index) then
                call func(curr_node%item, return_item)
                nullify (curr_node)
                return
            end if

            curr_node => curr_node%next
            index = index + 1
        end do
        nullify (curr_node)

end subroutine get_node_at_index_user
...

The user can use it like this:

program main

    use linked_list_m, only: child_list, rk, ik
    implicit none
    real(rk) :: x
    integer(ik) :: n

    type(this_child) :: list
    print *, list%size()

    call list%push(1.0_rk)
    call list%push(2_ik)

    call list%get(1, get_value, x)
    call list%get(2, get_value, n)

    print *, x, n, list%size()

contains

    subroutine get_value(this_item, return_item)
        class(*), intent(in) :: this_item
        class(*), intent(out) :: return_item

        select type (this_item)
        type is (real(rk))
            select type (return_item)
            type is (real(rk))
                return_item = this_item
            class default
                print *, '*<ERROR>* get_value: type mismatch'
            end select
        type is (integer(ik))
            select type (return_item)
            type is (integer(ik))
                return_item = this_item
            class default
                print *, '*<ERROR>* get_value: type mismatch'
            end select
        class default
            print *, "*<ERROR>* get_value: invalid type"
        end select

    end subroutine get_value

end program main


end do
nullify(current_node)
nullify(return_item)

This comment was marked as abuse.

Comment on lines +54 to +57
class(*), intent(in), optional :: new_item

! allocating new_item to the new node's item
allocate(new_node%item, source=new_item)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source uses the optional argument new_item here, there may be memory access errors.

@14NGiestas14NGiestas linked an issue Jul 5, 2022that may be closed by this pull request
@awvwgk

I guess this PR has become stale. Since we already have a decent implementation here, should we try to update the PR, address the review comments and get it merged?

@jvdp1

I guess this PR has become stale. Since we already have a decent implementation here, should we try to update the PR, address the review comments and get it merged?

Maybe @arjenmarkus / @chetan has some news/updates. Specs and tests are mainly missing.

@milancurcicmilancurcic mentioned this pull request Oct 18, 2022
@arjenmarkus

I have (finally, sigh ;)) started setting up a test program. The documentation will follow with it.

Sign up for free to join this conversation on . Already have an account? Sign in to comment
reviewers neededThis requires extra eyestopic: container(Abstract) data structures and containerswaiting for OPThis requires action from the OP
None yet

Successfully merging this pull request may close these issues.

Linked list