Click here to Skip to main content
12,404,478 members (69,433 online)
Rate this:
 
Please Sign up or sign in to vote.
See more: C
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:
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
idobry431
Rate this: bad
 
good
Please Sign up or sign in to vote.

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).
  Permalink  
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
Rate this: bad
 
good
Please Sign up or sign in to vote.

Solution 2

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!
  Permalink  
Comments
CPallini 2-Jan-13 5:44am
   
Good catch on i, my 5.
nv3 2-Jan-13 5:52am
   
Thanks!
Rate this: bad
 
good
Please Sign up or sign in to vote.

Solution 3

To state it bluntly: this code is crap.

My advise:
  1. 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)
  2. make all blocks { ... }
  3. do not rely on the comma operator (use ; - see 2. above)
  4. modify the iterating variables only at one place and not at several places (sc, i, len, res, len)
  5. 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
  Permalink  
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.

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

  Print Answers RSS
Top Experts
Last 24hrsThis month


Advertise | Privacy | Mobile
Web02 | 2.8.160721.1 | Last Updated 2 Jan 2013
Copyright © CodeProject, 1999-2016
All Rights Reserved. Terms of Service
Layout: fixed | fluid

CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100