2

I want to implement the sorted bag(collection) data structure(with a singly-linked list) in C++ and I have a problem when I want to test the add function. This is the test:

SortedBag sb(relation1);  (relation1 is e1<=e2) 
    sb.add(5);
    std::cout << sb.size()<<" ";
    sb.add(6);
    std::cout << sb.size() << " ";
    sb.add(0);
    std::cout << sb.size() << " ";
    sb.add(5);
    std::cout << sb.size() << " ";
    sb.add(10);
    std::cout << sb.size() << " ";
    sb.add(8);
    std::cout << sb.size() << " ";

And it will print 1 2 3 3 4 5 instead of 1 2 3 4 5 6.

This is the add function:

void SortedBag::add(TComp e) {
    Node* auxiliarElement = new Node;
    Node* CheckSLL = new Node;
    int flagStop = 1;

    if (this->head == nullptr)
    {
        auxiliarElement->value = e;
        auxiliarElement->freq = 1;
        auxiliarElement->next = nullptr;
        this->head = auxiliarElement;
    }
    else {
        CheckSLL = this->head;
        while (CheckSLL->next != nullptr && rel(CheckSLL->value, e)) 
        {
            if (CheckSLL->value == e) {
                CheckSLL->freq += 1;
                flagStop = 0;
                break;
            }
            CheckSLL = CheckSLL->next;
        }
        if (CheckSLL == this->head && flagStop)
        {
            auxiliarElement->value = e;
            auxiliarElement->freq = 1;
            auxiliarElement->next = this->head;
            this->head = auxiliarElement;
            flagStop = 0;
        }
        if (CheckSLL->value == e && flagStop)
        {
            CheckSLL->freq += 1;
            flagStop = 0;
        }
        if (flagStop) {
            auxiliarElement->value = e;
            auxiliarElement->freq = 1;
            auxiliarElement->next = nullptr;
            CheckSLL->next = auxiliarElement;
        }
    }
}

The size() functions works fine, I will post that too:

int SortedBag::size() const {
    int Size = 0;
    Node* goThrough = new Node;
    goThrough = this->head;
    while (goThrough != nullptr) {
        Size += goThrough->freq;
        goThrough = goThrough->next;
    }
    return Size;
}

And I can't find out why it doesn't add the frequency from the second 5. Can somebody help me, please? (the struct Node has value,freq and a pointer to the next Node)

Vlad from Moscow
  • 224,104
  • 15
  • 141
  • 268
Andrei Gabor
  • 141
  • 7
  • When you test this with a debugger, what does it do for the second 5 that you didn't expect? – Scott Hunter Apr 07 '20 at 18:11
  • Why not use `std::forward_list` as a starting point, and use that to implement the more complex data structure? You don't build your own hammer if you want to put together a table or chair. – PaulMcKenzie Apr 07 '20 at 18:11
  • I know man, but the teachers don't let us use STLs for this data structures course – Andrei Gabor Apr 07 '20 at 18:14
  • Even if the assignment restricts the use of `std::forward_list` it is in your interests to use `forward_list` as an aid while developing your program. You can write and test all of your non-list logic with a known-good list, and then write your own linked list and swap it in. This way you're developing and testing as little as possible at a time. – user4581301 Apr 07 '20 at 18:15
  • Note: You don't have to `new` storage every time you define a pointer. See this recent question:https://stackoverflow.com/questions/61086487/why-isnt-new-used-while-making-the-current-variable-in-the-linkedlist – user4581301 Apr 07 '20 at 18:18
  • @Gaboru Why is relation e1<=e2 and not e1 < e2 – Vlad from Moscow Apr 07 '20 at 18:18
  • Because that is the relation defined in the test for university – Andrei Gabor Apr 07 '20 at 18:19
  • It will print the same thing even for " – Andrei Gabor Apr 07 '20 at 18:21
  • 1
    Regardless, @VladfromMoscow 's point is important and can give you grief. See [Strict Weak Ordering](https://en.wikipedia.org/wiki/Weak_ordering) for details. – user4581301 Apr 07 '20 at 18:27
  • I suspect of `rel(CheckSLL->value, e)` what is that `rel`? It seems not to work well with zero values. Perhaps you want to initialize with some invalid (look for NaN) value so as to handle correctly zero-values. – Ripi2 Apr 07 '20 at 18:42
  • bool relation1(TComp e1, TComp e2) { return e1 <= e2; } – Andrei Gabor Apr 07 '20 at 18:44
  • and I have defined it as typedef bool(*Relation)(TComp, TComp); – Andrei Gabor Apr 07 '20 at 18:44
  • 1
    @Gaboru *Because that is the relation defined in the test for university* -- It may not have been explained fully, but that test is faulty. What do you return if `e1` and `e2` are equal? Let's assume you return `true`. Then let's say you get the same two equal items, but switched, i.e. `e2` and `e1`. If you return `true`, then your entire relation is ambiguous. How could `a` come before `b`, and at the same time `b` come before `a`? I wonder if your teacher also has put this test to see if your code falls apart if two items are equal. – PaulMcKenzie Apr 07 '20 at 22:45
  • Pretty weird that it adds the same element(grow the frequency) only for " – Andrei Gabor Apr 08 '20 at 13:46

1 Answers1

1

For starters these statements

Node* CheckSLL = new Node;

and

Node* goThrough = new Node;

result in memory leaks.

Also this output

And it will print 1 2 3 3 4 5.

does not correspond to the sequence of entered data because the function size counts the total value of frequencies

Size += goThrough->freq;

So as 6 elements were inserted in the list then the output should be

1 2 3 4 5 6

The relation should be specified like e1 < e2 not like e1 <= e2

The function add can be defined very simply. I assume that the relation corresponds to the operator <.

void SortedBag::add( TComp e ) 
{
    Node **current = &this->head;

    while ( *current != nullptr && rel( ( *current )->value, e ) )
    {
        current = &( *current )->next;
    }

    if ( *current == nullptr || rel( e, ( *current )->value ) )
    {
        Node *new_node = new Node;

        new_node->value = e;
        new_node->freq  = 1;
        new_node->next = *current;

        *current = new_node;
    }
    else
    {
        ++( *current )->freq;
    }
}

And you should decide whether the function size returns frequencies or the number of nodes in the list.

Here is a demonstrative program.

#include <iostream>
#include <functional>

template <typename T, typename Comparison = std::less<T>>
class List
{
private:
    struct Node
    {
        T value;
        size_t freq;
        Node *next;
    } *head = nullptr;

    Comparison cmp;

public:
    explicit List() : cmp( Comparison() )
    {
    }

    explicit List( Comparison cmp ) : cmp( cmp )
    {
    }

    ~List()
    {
        while ( this->head != nullptr )
        {
            Node *current = this->head;
            this->head = this->head->next;
            delete current;
        }
    }

    List( const List & ) = delete;
    List & operator =( const List & ) = delete;

    void add( const T &value );

    friend std::ostream & operator <<( std::ostream &os, const List &list )
    {
        for ( Node *current = list.head; current != nullptr; current = current->next )
        {
            os << current->value << ':' << current->freq << " -> ";
        }

        return os << "null";
    }
};


template <typename T, typename Comparison>
void List<T, Comparison>::add( const T &value ) 
{
    Node **current = &this->head;

    while ( *current != nullptr && cmp( ( *current )->value, value ) )
    {
        current = &( *current )->next;
    }

    if ( *current == nullptr || cmp( value, ( *current )->value ) )
    {
        Node *new_node = new Node { value, 1, *current };

        *current = new_node;
    }
    else
    {
        ++( *current )->freq;
    }
}

int main() 
{
    List<int> list;

    list.add( 5 );
    list.add( 6 );
    list.add( 0 );
    list.add( 5 );
    list.add( 10 );
    list.add( 8 );

    std::cout << list << '\n';

    return 0;
}

The program output is

0:1 -> 5:2 -> 6:1 -> 8:1 -> 10:1 -> null
Vlad from Moscow
  • 224,104
  • 15
  • 141
  • 268
  • Thank you! It's working now. But can you help me with the delete function too? I don't think it's hard – Andrei Gabor Apr 08 '20 at 13:46
  • @Gaboru You should select the best answer for this your question and ask a new question relative to the other problem. – Vlad from Moscow Apr 08 '20 at 13:48
  • Cand you help me make this to work even for the "<=" relation, please? I have more tests with that relation. What should I change in the add function? Others should look the same I think. – Andrei Gabor Apr 08 '20 at 15:19