Why my C++ code is not working

Question

1.00/5 (1 vote)

See more:

Hello
My c++ code is not working
There are no compilation error in my program.
The few lines of my code which asks the user to put value of T A and B is working but there output coming

Basically My code is to get the GCD of (A,B)

I have used Debugger but can not find the flaw.
Only thing I can find is that after reverse(line 27) function there is some error.

What I have tried:

C++

#include <iostream>
#include <vector>
#include <algorithm>
using namespace std;
void Divisor(int n,vector<int>vec)
{
    for(int i=1;i<n;i++)>
    {
        if(n%i==0)
        {
            vec.push_back(i);// It will be already sorted ha
        }
    }
}

int main()
{
    vector<int>vec1;
    int T;
    int A;
    int B;
    cin >> T;
    for(int j=0;j<t;++j)>
    {
        cin >> A >> B;

        Divisor(A,vec1);

        reverse(vec1.begin(),vec1.end());//LINE 27

        for(int k=0;k<vec1.size();k++)>
        {
            if(B%vec1.at(k)==0)
            {
                cout << vec1.at(k) << endl;
                break;
            }
        }
    }
}

Posted 6-Jul-16 0:17am

Member 12566145

Updated 6-Jul-16 4:53am

Richard MacCutchan

v2

Add a Solution

Comments

CPallini 6-Jul-16 6:43am

What is 'reverse'?

Patrice T 6-Jul-16 7:38am

Explain the error you get.

5 solutions

Solution 1

"there is some error" tells us nothing - anything could be happening and we have no idea what! Saying that is like your car breaking down in the middle of nowhere, then when you call the garage just saying "it broke!" and ending the call. How long do you think you will be waiting before they arrive with the right parts?

So use the debugger.
Put a breakpoint at the start of the function, run your app in the debugger, and step through each line in turn. Use the debugger to look at what is in what variable.
Before you execute each line, work out what you expect to happen first, then execute the line and see what did happen. Was it what you expected? If so, move on. If not ... why not? What was different? What happened that you didn't expect, or didn't happen that you did?

Debugging is a skill: you only develop it by using it. And it's a lot better to develop it on a trivial code sample like this than on a 100,000 line complete project!
Give it a try: see what information you can find out!

Posted 6-Jul-16 0:44am

OriginalGriff

Solution 2

The way you calc the GCD is complicated and a waste of resources.
Collecting all divisors of a number takes a lot of time.

Bug: You forgot that n is divisor of itself !

C++

void Divisor(int n,vector<int>vec)
{
    for(int i=1; i<n>+1; i++)>
    {
        if(n%i==0)
        {
            vec.push_back(i);// It will be already sorted ha
        }
    }
}</n></int>

Greatest common divisor - Wikipedia, the free encyclopedia[^]

[UpDate]
The problem with your program is that the bigger the numbers, longer it takes to get the answer. It is O(n).
The right way to do it is by taking advantage of the mathematical relation between the 2 numbers and the GCD.
The 2 numbers are multiples of the GCD, it imply that the difference between the 2 numbers is also a multiple of the GCD.
This means that GCD(A,B) <=> GCD(B, A%B)
This

C++

int GCD(int A, int B)
{
    int C;
    C= A % B;
    if (C == 0)
        return B;
    return GCD(B, C);
}

is a recursive version.

Posted 6-Jul-16 1:57am

Patrice T

Updated 6-Jul-16 9:15am

v3

Solution 5

reverse - C++ Reference[^]

Posted 6-Jul-16 4:53am

Vignesh Mani

Solution 4

Try this one...

#include <iostream>
#include <vector>
#include <algorithm>
using namespace std;
void Divisor(int n,vector<int>&vec)
{
    for(int i=n;i>0;i--)
    {
        if(n%i==0)
        {
            vec.push_back(i);            
        }
    }   
}
 
int main()
{
    vector<int>vec1;
    int T;
    int A;
    int B;
    cin >> T;
    for(int j=0;j<t;++j)>
    {
        cin >> A >> B; 
       Divisor(A,vec1);
        for(int k=0;k<vec1.size();k++)>
        {
            if(B%vec1.at(k)==0)
            {
                cout << vec1.at(k) << endl;
                break;
            }
        }
    }
}

Posted 6-Jul-16 3:56am

Member 12346239

Updated 6-Jul-16 4:08am

v3

Comments

Richard MacCutchan 6-Jul-16 10:03am

If you are going to post code, then please format it properly.

Leo Chapiro 6-Jul-16 10:27am

Is it not the same what has sudgested Jochen Arndt in his Solution 3?

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Jochen Arndt · Accepted Answer · 2016-07-06T02:07:00

You are passing the vector by value to your Divisor function:

void Divisor(int n, vector<int> vec)

When calling the function, a new vector instance is created and filled with the data of the passed vector. But the content of the passed vector is not changed.

To change the content of the passed vector, pass it by pointer or reference:

void Divisor(int n, vector<int>& vec)