Loop condition problem (for)

Question

0.00/5 (No votes)

See more:

Hi guys
im trying to go out from the loop when my string=0(flase)but from some reason it dosn't work.
this is my function:

C#

int getArchiveLength(char * str)
{
    int sc,i,count=1,len=1,res;
    for(sc=0,i=1;str[sc];sc++,len++)
    {
        while(str[sc]==str[sc+i]) count++,i++;
        if(count>=9)
        {
            res=count/9;
            len=((res*2)+1);
        }
        else
            len++;
        sc+=i; count=0;
    }
    return len;
}

Posted 1-Jan-13 23:17pm

idobry

Add a Solution

3 solutions

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)

CPallini · Answer 1 · 2013-01-01T23:30:00

Solution 1

Quote:
while(str[sc]==str[sc+i]) count++,i++;

When a match is not found, you increment twice sc (because i is 1).
I suppose

sc+=i-1;

(instead of sc+=i;) would fix it).

Posted 1-Jan-13 23:30pm

CPallini

Comments

idobry 2-Jan-13 5:36am

I do this sc+=i; becuse i want to keep checking the string from the last char.
when i debugger it i really see that str[sc]=0 but it dosn't comes out from the loop
that what freaking me out

nv3 · Answer 2 · 2013-01-01T23:42:00

There are several things wrong here:

i is initialized only once at the start of the for loop. But it should be initialized inside the loop.

res is being set, but never used.

len is being calculated wrong. Even for an empty string a value of 1 is being returned.

You are pasting this code now for the second time. Instead, you should use a debugger and step through your code, as I have recommended already before. If you think you can get away without learning how to use a debugger you won't get far in software development. Some advice: Single-step through every new piece of code to see if everything is working as expectedly!

Andreas Gieriet · Answer 3 · 2013-01-02T00:45:00

Solution 3

To state it bluntly: this code is crap.

My advise:

describe in a comment what the function aims to calculate (it seems to be some length encoding compression algorithm, but it's not clear how exaclty)
make all blocks { ... }
do not rely on the comma operator (use ; - see 2. above)
modify the iterating variables only at one place and not at several places (sc, i, len, res, len)
do consistent initialisation (e.g. not clear why initially count = 1 and later you do count = 0 etc.)

Try to describe the function, define some unit tests and finally properly re-implement it.

Cheers
Andi

Posted 2-Jan-13 0:45am

Andreas Gieriet

Updated 2-Jan-13 0:47am

v3

Comments

nv3 2-Jan-13 7:04am

You are so right! The only excuse is that he is a beginner and obviously still fighting with the basics.

Andreas Gieriet 2-Jan-13 9:24am

I do not want to blame him.
The code is simply too obscure to maintain - as he did experience... ;-)
If he describes what he wants to achieve, he will for sure get some concrete help to re-implement his function.
Maybe, the following is a starting point:

int getArchiveLength(const char* str)
{
	int len = 0;
	int inputBlockLength = 1; // block = sequence of consecutive identical input characters
	int compressedBlockLength = 0; // 1...9 characters = plain text, longer sequences = ???
	for(const char *p = str; *p; p += inputBlockLength)
	{
		// count number of consecutive identical characters
		while(*p == p[inputBlockLength])
		{
			inputBlockLength++;
		}
		// calculate compressed lenght per block
		if (inputBlockLength == 1)
		{
			compressedBlockLength = inputBlockLength;
		}
		else if (inputBlockLength < 9)
		{
			compressedBlockLength = inputBlockLength; // --> dubious?
		}
		else
		{
			compressedBlockLength = 1 + 2 * (inputBlockLength / 9); // --> dubious?
		}
		len += compressedBlockLength;
	}
	return len;
}

Cheers
Andi

nv3 2-Jan-13 10:03am

I guess from a previous post of OP that he is trying to do the following kind of compression:

Input: ABBCCCCDDDDDDDDDEEEEEEEEEEEE
Output: A1B2C4D9E9E3

And the output of his getArchiveLength function should reflect the length of the compressed string. Perhaps OP can confirm that this is what he really wants?

Andreas Gieriet 2-Jan-13 10:51am

Ok, if so, I'd suggest something like this:

// each block of 1...MaxBlockLen identical consecutive characters encodes into EncodedBlockLen characters.
// E.g. for MaxBlockLen = 9, EncodedBlockLen = 2:  ABBCCCDDDDDDDDDDDDDDEE = A1B2C3D9D5E2
int getArchiveLength(const char* str)
{
   const int MaxBlockLen = 9;
   const int EncodedBlockLen = 2;

   int outLen = 0;
   int blockLen = 1;
   // loop over the input blocks: each block consists of 1...MaxBlockLen identical characters
   for(const char *p = str; *p; p += blockLen)
   {
      blockLen = 1;
      while((p[0] == p[blockLen]) && (blockLen < MaxBlockLen))
      {
         blockLen++;
      }
      outLen += EncodedBlockLen;
   }
   return outLen;
}

Cheers
Andi

PS: Next step was to write unit tests for this and, if needed, to optimize (do some x / 9 and x % 9 stuff, etc.).

nv3 2-Jan-13 11:01am

Except for

outLen += EncodedBlockLen; // replace by:
outlen += blockLen + 1;

that looks good to me. I hope OP sees it and makes use of your code.

Andreas Gieriet 3-Jan-13 16:43pm

I think your suggested change is not correct.
The input string is chopped up into blocks of 1 to 9 identical characters. Each block translates into exactly 2 characters each: one for the block character and one for the number of identical characters (1...9).
Cheers
Andi

nv3 3-Jan-13 18:08pm

You are so right, Andreas. I just mixed up the block length and the encoded length.