Prints numbers, but can't get rid of comma on last number

[Yezman]

So here is what I have.

(Note I did add a swap in for if end < start)

//Requires: start > 0; end > 0; 0 <= digit <= 9
//Modifies: nothing
//Effects: runs the range of numbers from start to end inclusive
// if (end < start), it will swap start and end
// prints the number if any digit of the number is the
// same as 'digit'
// printIfHoldsDigit(5, 10, 7) //prints: 7
// printIfHoldsDigit(5, 30, 8) //prints: 8, 18, 28
// printIfHoldsDigit(1, 30, 2) prints: 2, 12, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29

void printIfHoldsDigit(int start, int end, int digit)
{ 
		for(start = start; start <= end; start++)
		{
			if (equalsDigit(start,digit) == true)
			{
				cout << start << ", ";
			}
		}
}

equalsDigit just returns true if a number holds a digit.

My issue is on my last print I can not have the ", " printing.

Anyone have an suggestions as to how to solve this issue.

[webJose]

const char *separator = "";
for (; start < end; start++)
{
    if (equalsDigit(start, digit))
    {
        cout << separator << start;
        separator = ", ";
    }
}

[shacktar]

(1) You can use std::string to append the output. Then, when it comes to print the string, you can print the substring up to but not including the last two characters. You can check the string's length to see if there's anything to output.

(2) You can do one pass to figure out which is the last number you will be outputting. Then, on the second pass you can output the comma only if the number you're currently printing is not the last one.

[Yezman]

I ended up getting it after reading both posts. Thank you!

void printIfHoldsDigit(int start, int end, int digit)
{
	if (start > end)
	{swap(start,end);}

	int first = 2;
	for(start = start; start <= end; start++)
		{
			if (equalsDigit(start,digit) && first == 2)
			{
				first = 3;
				cout << start;
			}
			else if (equalsDigit(start,digit) && first == 3)
			{
				cout << ", " << start;
			}
		}
}

[shacktar]

You don't need the two if statements in there. In the loop you can just do this:

if(equalsDigit(start,digit))
{
    if(!first)
    {
        cout << ", ";
    }
    
    cout << start;
    first = false;
}

though webJose's solution is more efficient.

[Yezman]

Ah that makes sense... Actually I just looked at my function above this one on my project and I did the same thing (used 2 if's when only one is needed). I guess thats a habbit I have to break now while I am still learning.

[Duthomhas]

How is it more efficient?

The most efficient version (not yet presented) avoids all the checks/assignments by simply printing the first element, and only then entering a loop to print the remainder.

[webJose]

Mine's probably more efficient because there are no IF's, which I assume take a few processor cycles. Just a guess. I don't really know if those save cycles are wasted in the assignation of the separator. Would be nice if someone compiled and then showed the assembly (a topic I know nothing about).

[Duthomhas]

Yours is less efficient because a test and branch only take a few cycles, whereas a traverse and allocate take considerably more.

[shacktar]

The most efficient version (not yet presented) avoids all the checks/assignments by simply printing the first element, and only then entering a loop to print the remainder.

I think here we'd have to have one loop to find and print the first element and then one loop to find and print the rest.

[Duthomhas]

shacktar wrote:

I think here we'd have to have one loop to find and print the first element and then one loop to find and print the rest.

Yes, exactly. This is called "loop unrolling" in optimization techniques -- you are taking a condition out of the loop and simply having more code to do the thing. This is a special case of that, where you perform (or don't perform) something in a fixed iteration (or iterations) of the loop.