If i run this program in codeblocks i didn't get a proper ans, but if i debug it then i get proper ans. it's hapenning round and round

Question

This problem is about adding two sparse matrix. At first I created two sparse matrix s1,s2 (taking the elements value manually) then adding them in sum. If I run it in code blocks I got an incorrect answer. But if I debug and run it, then the answer I got is perfect. This thing is happening again and again. I don't know why. Please give me a fruitful solution. Thank you

#include<bits/stdc++.h>
using namespace std;
struct Element
{
    int i;
    int j;
    int x;
};
struct Sparse_Matrix
{
    int m;
    int n;
    int num;
    struct Element *e;
};
void creation(struct Sparse_Matrix *s)
{
    cout<<"Enter dimension : ";
    cin>>s->m>>s->n;
    cout<<"Enter the number of nonzero elements : ";
    cin>>s->num;
    s->e=new Element[s->num+1];
    cout<<"Enter all non zero elements with it's location "<<endl;
    for(int p=1; p<=s->num; p++)
        cin>>s->e[p].i>>s->e[p].j>>s->e[p].x;
}
struct Sparse_Matrix *Addition(struct Sparse_Matrix *s1,struct Sparse_Matrix *s2)
{
    if((s1->m != s2->m) && (s1->n != s2->n))
        return 0;
    struct Sparse_Matrix *sum;
    sum=new Sparse_Matrix();
    sum->e=new Element[s1->num+s2->num+1];
    int i=0,j=0,k=0;
    while(i<=s1->num && j<=s1->num)
    {
        if(s1->e[i].i<s2->e[j].i)
            sum->e[k++]=s1->e[i++];
        else if(s1->e[i].i>s2->e[j].i)
            sum->e[k++]=s2->e[j++];
        else
        {
            if(s1->e[i].j<s2->e[j].j)
                sum->e[k++]=s1->e[i++];
            else if(s1->e[i].j>s2->e[j].j)
                sum->e[k++]=s2->e[j++];
            else
            {
                sum->e[k]=s1->e[i++];
                sum->e[k++].x+=s2->e[j++].x;
            }
        }
    }
    for(; i<=s1->num; i++)
        sum->e[k++]=s1->e[i++];
    for(; j<=s2->num; j++)
        sum->e[k++]=s2->e[j++];
    sum->n=k;
    sum->m=s1->m;
    sum->n=s1->n;
    return sum;
};
void display(struct Sparse_Matrix s)
{
    int k=1;
    for(int p=1; p<=s.m; p++)
    {
        for(int q=1; q<=s.n; q++)
        {
            if(s.e[k].i==p && s.e[k].j==q)
                cout<<s.e[k++].x<<" ";
            else
                cout<<"0 ";
        }
        cout<<endl;
    }
}
int main()
{
    struct Sparse_Matrix s1,s2,*sum;
    creation(&s1);
    creation(&s2);
    cout<<"Displaying the s1 Matrix "<<endl;
    display(s1);
    cout<<"Display the s2 Matrix "<<endl;
    display(s2);
    sum=Addition(&s1,&s2);
    cout<<"Displaying the sum "<<endl;
    display(*sum);
}

OT: `struct Sparse_Matrix s1,s2,*sum;` in `c++` you don't need to use `struct` on ths line. `Sparse_Matrix s1,s2,*sum;` is sufficient. — drescherjm, Jul 23 '21 at 18:32
`void display(struct Sparse_Matrix s)` do you really want to pass by value here instead of by const reference? — drescherjm, Jul 23 '21 at 18:33
Debuggers often run a simpler, more literal interpretation of the code than a release build that spent time optimizing the code. Sometimes this works against you and hides bugs. — user4581301, Jul 23 '21 at 18:34
Your code has several memory leaks. Although when you fix them you will run into double deletes because of the lack of handling the rule of 3/5/0 combined by using pass by value. — drescherjm, Jul 23 '21 at 18:35
When facing a mystery bug, one of the first things you should do is remove things known to cause mystery bugs like [`#include`](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) and [`using namespace std;`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) and see what happens without them. — user4581301, Jul 23 '21 at 18:36
Also consider replacing the `new`ed arrays with `std::vector` and taking advantage of the `at` method's bounds checking. If the code now throws an exception, you'll have found a bug. — user4581301, Jul 23 '21 at 18:38
Do you want C or C++. Because, as others already clarified, in C++ you must not code like this. If this is C, you still shouldn't code like this. Memory management requires rigor in both languages. — Costantino Grana, Jul 23 '21 at 19:02
Also chose if you want 1-based or 0-based indexing. In `creation` and `display` you use 1-based indexing, while in `Addition` it's 0-based. You will end up with one element more than expected, without initialization. — Costantino Grana, Jul 23 '21 at 19:10
So probably you can fix this by setting `i`, `j`, `k` to 1. But you still leak like crazy. — Costantino Grana, Jul 23 '21 at 19:11
Indexes in C and C++ are in range of `0` and `n - 1` not `1` to `n`! — Marek R, Jul 23 '21 at 19:13
@MarekR You are definitely right, but I still can understand someone learning which prefers 1-based indexing to keep things simple. But at least be consistent. Moreover a few algorithms (like Fenwick trees) get a much simpler computation starting from 1. Sparse matrices are not the case, anyway... — Costantino Grana, Jul 23 '21 at 20:29
You also have double increment in the final for loops of the addition. — Costantino Grana, Jul 23 '21 at 21:15

score 1 · Answer 1 · answered Jul 23 '21 at 21:34

Ok, let's try to put a little C++ in that code (while fixing the many bugs):

#include <vector>
#include <iostream>
#include <exception>
#include <sstream>

struct SparseMatrix {
    struct Element {
        int i;
        int j;
        int x;

        friend std::istream& operator>>(std::istream& is, SparseMatrix::Element& e) {
            return is >> e.i >> e.j >> e.x;
        }

        bool operator<(const Element& other) const {
            if (i < other.i) {
                return true;
            }
            else if (other.i < i) {
                return false;
            }
            else if (j < other.j) {
                return true;
            }
            else if (other.j < j) {
                return false;
            }
            else {
                return false;
            } 
        }
    };

    int m_; // rows
    int n_; // cols
    std::vector<Element> e_;

    SparseMatrix(int m = 0, int n = 0) : m_(m), n_(n) {}
    Element& operator[](size_t pos) { return e_[pos]; }
    const Element& operator[](size_t pos) const { return e_[pos]; }
    void push_back(const Element& val) { e_.push_back(val); }
    int rows() const { return m_; }
    int cols() const { return n_; }
    // Non Zero Elements
    size_t nze() const { return e_.size(); }

    friend SparseMatrix Add(const SparseMatrix& s1, const SparseMatrix& s2);

    friend std::istream& operator>>(std::istream& is, SparseMatrix& sm) {
        size_t num;
        is >> sm.m_ >> sm.n_ >> num;
        SparseMatrix::Element e;
        for (size_t i = 0; i < num; ++i) {
            is >> e;
            sm.push_back(e);
        }
        return is;
    }

    friend std::ostream& operator<<(std::ostream& os, const SparseMatrix& sm) {
        size_t k = 0;
        for (int r = 0; r < sm.rows(); r++) {
            for (int c = 0; c < sm.cols(); c++) {
                if (sm[k].i == r && sm[k].j == c)
                    os << sm[k++].x << " ";
                else
                    os << "0 ";
            }
            os << '\n';
        }
        return os;
    }
};

SparseMatrix Add(const SparseMatrix& s1, const SparseMatrix& s2)
{
    if (s1.rows() != s2.rows() || s1.cols() != s2.cols()) {
        throw std::exception("Dimension mismatch");
    }

    SparseMatrix sum(s1.rows(), s1.cols());
    sum.e_.resize(s1.nze() + s2.nze());
    size_t i = 0, j = 0, k = 0;
    while (i < s1.nze() && j < s1.nze()) {
        auto e1 = s1[i], e2 = s2[j];
        if (e1 < e2) {
            sum[k++] = e1;
            i++;
        }
        else if (e2 < e1) {
            sum[k++] = e2;
            j++;
        }
        else {
            sum[k] = e1;
            sum[k++].x += e2.x;
            i++;
            j++;
        }
    }
    while (i < s1.nze())
        sum[k++] = s1[i++];
    while (j < s2.nze())
        sum[k++] = s2[j++];
    return sum;
};

int main()
{
    SparseMatrix s1, s2;
    std::stringstream in(R"(
5 7
4
1 4 7
3 4 1
4 2 3
4 6 8

5 7
3
2 3 5
3 2 8
4 6 1
    )");
    in >> s1 >> s2;
    std::cout << "Displaying the s1 Matrix\n" << s1 << '\n';
    std::cout << "Displaying the s2 Matrix\n" << s2 << '\n';
    SparseMatrix sum = Add(s1, s2);
    std::cout << "Displaying the sum\n" << sum << '\n';

    return 0;
}

The errors were here:

    if((s1->m != s2->m) && (s1->n != s2->n))
        return 0;

where you want to stop if either rows or column differ. Use an or (||). Another error was here:

    for(; i<=s1->num; i++)
        sum->e[k++]=s1->e[i++];
    for(; j<=s2->num; j++)
        sum->e[k++]=s2->e[j++];

in fact i and j get incremented twice in the loop. Just switch to while.

If i run this program in codeblocks i didn't get a proper ans, but if i debug it then i get proper ans. it's hapenning round and round

1 Answers1