I keep being off by one

[357mag 3]

It seems I'm always off by one. This program changes the contents of an array using a function and a pointer. The problem is the while loop in the function. At some point the value of num has to become 0 for the loop to fail and stop printing numbers. But on paper when I plot this thing out, I have already printed all 10 squares 1 through 10, but the value in num is still 1.

 

Shouldn't at that point the value of num be 0? But I keep getting 1. So it's like all 10 squares have been printed but the condition in my while loop looks like this:

 

while(1)

 

instead of...

 

while(0)

 

Here is the code. Sorry I have not changed the namespace stuff yet:

void square(int *n, int num);

int main()
{
    int i, nums[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

    cout << "Original array values: ";

    for(i = 0; i < 10; i++)
        cout << nums[i] << " ";

    cout << endl;

    square(nums, 10);

    cout << "Altered array values: ";

    for(i = 0; i < 10; i++)
        cout << nums[i] << " ";

    cout << endl << endl;

    system("pause");
    return 0;
}

void square(int *n, int num)
{
    while(num)
    {
        *n = *n * *n;
        num--;
        n++;
    }
}

 

[357mag 3]

Plus I was trying to use the debugger so I could watch the value of num in the while loop but there is no step into command. Just step over and trace into. Actually, I've never used a debugger, so I'm new to that. I'm pretty close on this maybe my paperwork is just a bit off.

Edited January 27 by 357mag

[Anders Melander 1984]

5 hours ago, 357mag said:

Just step over and trace into.

Trace Into = Step Into

 

But you don't need to trace into. You can just place a breakpoint inside your square function.

 

This is a very simple problem so I suggest you try to single-step through it "in your head" first; What happens when you call square(0), square(1), etc.

[David Heffernan 2433]

6 hours ago, 357mag said:

Plus I was trying to use the debugger so I could watch the value of num in the while loop but there is no step into command. Just step over and trace into. Actually, I've never used a debugger, so I'm new to that. I'm pretty close on this maybe my paperwork is just a bit off.

Even if you don't use a debugger, get your program to print out the intermediate values in the function square. Then compare it to your expectation. 

[Remy Lebeau 1577]

@357mag

 I don't understand what you are describing. Of course num reaches 0 or else the function would never exit. 

 

Proof:

https://onlinegdb.com/bxMEvMyoc

[357mag 3]

Yes num reaches 0. I'm just saying when I put all this stuff on paper, it seems after the last square has been printed, there still is one iteration left to go. So I end up with while(1) instead of while(0). But it's possible that my paperwork was just off a little. I understand how the program works.

[357mag 3]

I can't use Trace Into because this is what I see when I use it:

 

 

[Remy Lebeau 1577]

9 minutes ago, 357mag said:

I'm just saying when I put all this stuff on paper, it seems after the last square has been printed, there still is one iteration left to go. So I end up with while(1) instead of while(0).

That makes no sense. You should show your paper logic. 

Quote

But it's possible that my paperwork was just off a little.

Entry into function:

num=10

 

while (10)

square 1

num=9

 

while (9)

square 2

num=8

 

while (8)

square 3

num=7

 

while (7)

square 4

num=6

 

while (6)

square 5

num=5

 

while (5)

square 6

num=4

 

while (4)

square 7

num=3

 

while (3)

square 8

num=2

 

while (2)

square 9

num=1

 

while (1)

square 10

num=0

 

while (0)

break

 

10 iterations total, and num is 0 after the last iteration. It can't be 1. 

Edited January 27 by Remy Lebeau

[Remy Lebeau 1577]

17 minutes ago, 357mag said:

I can't use Trace Into because this is what I see when I use it:

Are you saying you get that when you put a breakpoint on the call to square() and then step into it?  It should not be stepping into std code, let alone ntdll code.  Are you sure you are not maybe stepping into the std::cout calls instead?

[357mag 3]

I put my breakpoint on the function header:

 

void square...

 

...and when I pressed Trace Into I got all that crazy stuff that looked like addresses.

[Remy Lebeau 1577]

1 hour ago, 357mag said:

I put my breakpoint on the function header:

 

void square...

You can't put a breakpoint on a function declaration.  Put the breakpoint inside of your main() function instead, at the spot where square() is actually being called:

int main()
{
    // ...

    square(nums, 10); // <-- breakpoint here!!!

    //...
}

Now you should be able to step into square() at runtime.

 

Or, simply put the breakpoint inside of square() itself:

void square(int *n, int num)
{
    while(num) // <-- breakpoint here!!!
    {
        *n = *n * *n;
        num--;
        n++;
    }
}

 

Edited January 27 by Remy Lebeau

[357mag 3]

Same thing happens. If I put the breakpoint next to while(num) and hit F7, the same window with all those crazy addressess shows up. If I put the breakpoint next to the function call square(nums) I get this screen:

 

 

[357mag 3]

I have been able to successfully step into or trace into but I can't recall how I did it. 

[Remy Lebeau 1577]

4 hours ago, 357mag said:

If I put the breakpoint next to while(num) and hit F7, the same window with all those crazy addressess shows up. If I put the breakpoint next to the function call square(nums) I get this screen

Sounds like a corrupted build or environment.  If you start a new project fresh, do you get the same problem? If so, you might try reinstalling the IDE. 

[Rick Malik 0]

But aren't there 11 nums? 0 thru 10 = 11

[Remy Lebeau 1577]

24 minutes ago, Rick Malik said:

But aren't there 11 nums? 0 thru 10 = 11

No. There are only 10 integers declared in the array (values 1..10). Valid indexes into the array are only 0..9, inclusive.

Edited 23 hours ago by Remy Lebeau

[Rick Malik 0]

Okay so I tried this

 

I put this in the .h

int nums,num;
int __stdcall  square(int *n, int num);

and this in the .cpp

void __fastcall TForm1::Button1Click(TObject *Sender)
{
    int i, nums[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    for(i = 0; i < 10; i++)
    square(nums, 10);
    for(i = 0; i < 10; i++)
    Label1->Caption = IntToStr(nums);
    return ;
}
//---------------------------------------------------------------------------
int __stdcall TForm1:: square(int *n, int num)
{
    while(num)
    {
        *n = *n * *n;
        num--;
        n++;
    }
    Label2->Caption = IntToStr(num);
}

Label1 says 1
Label2 says 0

am I doing this right?

[Remy Lebeau 1577]

2 hours ago, Rick Malik said:

I put this in the .h

int nums,num;

Why? Those variables are not being used anywhere, so there is no point in declaring them there.

2 hours ago, Rick Malik said:

    for(i = 0; i < 10; i++)
    square(nums, 10);

Why are you calling the function 10 times? The original OP code called it only 1 time.

 

Each time the function is called, the contents of the array are altered, but your code is not using the array values for anything, so there's really no point in altering the array at all.  At least the OP's code was outputting the original values and then the altered values.  You are not doing that.

2 hours ago, Rick Malik said:

    for(i = 0; i < 10; i++)
    Label1->Caption = IntToStr(nums);

Why are you updating the Label 10 times with the same value?  Perhaps you meant to display nums[ i ] instead?  But then you would end up displaying only the last integer in the array.  If you want to display all 10 values at one time, this is not the correct code for that task.

2 hours ago, Rick Malik said:

Label1 says 1
Label2 says 0

am I doing this right?

You are updating the Labels in different places using different variables that have different values, so of course the results are going to be different.

 

When updating Label1, you are using the nums variable that is local to the Button1Click() method.  For starters, that code should not even compile as shown, as nums is an array, but the Sysutils::IntToStr() function does not accept an array as input,  If this is indeed your actual code, the only way it could work is if the compiler decays the array into an int* pointer to the 1st array element, and then implicitly converts that pointer into a boolean, and then implicitly converts that boolean into an integer. That is too many implicit conversions for one statement, per the C++ standard.  But, lets assume your compiler ignores that fact, then that would explain how you end up with a final output of "1", as a non-null pointer converts to a boolean true, and true converts to an integer 1.

 

When updating Label2, you are using the num parameter that is local to the square() method, which was decremented to 0 while the loop ran.

 

If you really want to replicate the OP's code but display the results in a GUI instead of a console, then try something more like this instead:

class TForm1 : public TForm
{
    TButton *Button1;
    TLabel *Label1;
    TLabel *Label2;
    void __fastcall Button1Click(TObject *Sender);
private:
    void square(int *n, int num);
};

void __fastcall TForm1::Button1Click(TObject *Sender)
{
    int i, nums[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    String s;

    s = _D("Original array values:");
    for(i = 0; i < 10; ++i)
        s += (_D(" ") + IntToStr(nums[i]));

    Label1->Caption = s;

    square(nums, 10);

    s = _D("Altered array values:");
    for(i = 0; i < 10; ++i)
        s += (_D(" ") + IntToStr(nums[i]));

    Label2->Caption = s;
}
//---------------------------------------------------------------------------
void TForm1::square(int *n, int num)
{
    while(num)
    {
        *n = *n * *n;
        num--;
        n++;
    }
}
//---------------------------------------------------------------------------

 

Edited 19 hours ago by Remy Lebeau

[Rick Malik 0]

Pretty funny how  you react to this stuff....

BTW, your code wouldn't compile . Here are the errors from the compiler:

Build
  [C++ Error] Unit1.cpp(28): E2268 Call to undefined function '_D'
  [C++ Error] Unit1.cpp(30): E2015 Ambiguity between '_fastcall System::operator +(int,const System::Variant &)' and '_fastcall System::operator +(int,const System::Currency &)'
  [C++ Error] Unit1.cpp(38): E2015 Ambiguity between '_fastcall System::operator +(int,const System::Variant &)' and '_fastcall System::operator +(int,const System::Currency &)'
  [C++ Warning] Unit1.cpp(55): W8070 Function should return a value
 

So why num and nums are in the header is so they wouldn't fall out of scope, as I placed them under a second button. This time they both came out zeros(?)

 

And I didn't try to rewrite his method, I simply removed the couts so I could just run the function(). 

 

But thanks for your input. It was more than enlightening.

//----------------------------------------------------
void __fastcall TForm1::Button1Click(TObject *Sender)
{
    int i, nums[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    for(i = 0; i < 10; i++)
    square(nums, 10);
    for(i = 0; i < 10; i++)
    return ;
}
//---------------------------------------------------
int __stdcall TForm1:: square(int *n, int num)
{
    while(num)
    {
        *n = *n * *n;
        num--;
        n++;
    }
    return num;
}
//----------------------------------------------------
void __fastcall TForm1::Button2Click(TObject *Sender)
{
Label1->Caption = IntToStr(nums);
Label2->Caption = IntToStr(num);
}

[Remy Lebeau 1577]

8 hours ago, Rick Malik said:

BTW, your code wouldn't compile

It compiles and runs perfectly fine for me.

8 hours ago, Rick Malik said:

  [C++ Error] Unit1.cpp(28): E2268 Call to undefined function '_D'

What version of C++Builder are you using?  The _D() macro is defined in sysmac.h (a core system header) and has been available since C++Builder 2009 when the  RTL switched to Unicode.  The macro ensures a string/character literal matches the same data type used by System::Char depending on which platform you are compiling for - wchar_t on Windows and char16_t on Posix.

8 hours ago, Rick Malik said:

  [C++ Error] Unit1.cpp(30): E2015 Ambiguity between '_fastcall System::operator +(int,const System::Variant &)' and '_fastcall System::operator +(int,const System::Currency &)'
  [C++ Error] Unit1.cpp(38): E2015 Ambiguity between '_fastcall System::operator +(int,const System::Variant &)' and '_fastcall System::operator +(int,const System::Currency &)'

There are no Variant's or Currency's in this code, so you should not be getting those errors.  The only use of  operator+ is to concatenate a string literal with a System::String, and that is a well-defined operation.

 

I'm starting to think you are trying to run VCL GUI code in a project that does not actually have the VCL enabled properly.

8 hours ago, Rick Malik said:

  [C++ Warning] Unit1.cpp(55): W8070 Function should return a value

That is because you declared the square() method as having an 'int' return type, but there was no 'return' statement inside the method's body.  If you look at the code I gave you, I declared square() with a 'void' return type instead. Because there is no value worth returning to the caller.

8 hours ago, Rick Malik said:

So why num and nums are in the header is so they wouldn't fall out of scope

But, you are not USING THEM for anything, they are just sitting there.  The rest of your code uses only LOCAL VARIABLES of the same names, which shadow the class members,

8 hours ago, Rick Malik said:

as I placed them under a second button. This time they both came out zeros(?)

All data members of a TObject-derived class (like TForm is) are initialized to zeroes before the constructor is called.  And then you were not assigning any new values to them afterwards.

8 hours ago, Rick Malik said:

And I didn't try to rewrite his method

But you did rewrite it nonetheless, even if unintentionally.

8 hours ago, Rick Malik said:

I simply removed the couts so I could just run the function(). 

Which is exactly what the code I gave you does.

8 hours ago, Rick Malik said:

void __fastcall TForm1::Button1Click(TObject *Sender)
{
    int i, nums[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    for(i = 0; i < 10; i++)
    square(nums, 10);
    for(i = 0; i < 10; i++)
    return ;
}

This code is written wrong.  You removed the couts, but you did not remove the loops, too.  You are calling square() inside the 1st loop, and calling 'return' in the 2nd loop.  So, this code behaves as-if you had written it like this:

void __fastcall TForm1::Button1Click(TObject *Sender)
{
    int i, nums[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    for(i = 0; i < 10; i++) {
        square(nums, 10);
    }
    for(i = 0; i < 10; i++) {
        return;
    }
}

8 hours ago, Rick Malik said:

int __stdcall TForm1:: square(int *n, int num)
{
    while(num)
    {
        *n = *n * *n;
        num--;
        n++;
    }
    return num;
}

You are using the num variable that is local to the square() method, not the num variable that is declared as a member of the TForm1 class.  And num is ALWAYS 0 when the 'return' statement is reached.

8 hours ago, Rick Malik said:

void __fastcall TForm1::Button2Click(TObject *Sender)
{
Label1->Caption = IntToStr(nums);
Label2->Caption = IntToStr(num);
}

This code is displaying the values of the class members, which you are NEVER assigning any values to, so they just have their initial values of 0.

 

If you really want to use class members, then the code should look something more like this:

class TForm1 : public TForm
{
__published:
	TButton *Button1;
	TButton *Button2;
	TLabel *Label1;
	TLabel *Label2;
	void __fastcall Button1Click(TObject *Sender);
	void __fastcall Button2Click(TObject *Sender);
private:	// User declarations
	int nums[10], num;
	void square();
    ...
};

void __fastcall TForm1::Button1Click(TObject *Sender)
{
	for(int i = 0; i < 10; ++i) {
		nums[i] = i+1;
	}

	num = 10;
	square();
}
//---------------------------------------------------------------------------
void TForm1::square()
{
	int *n = nums;
	while(num)
	{
		*n = *n * *n;
		num--;
		n++;
	}
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button2Click(TObject *Sender)
{
	String s = _D("Array values:");
	for(int i = 0; i < 10; ++i) {
		s += (_D(" ") + IntToStr(nums[i]));
	}

	Label1->Caption = s;
	Label2->Caption = IntToStr(num);
}
//---------------------------------------------------------------------------