Click here to Skip to main content
Rate this: bad
good
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
idobry388
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 at 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 at 2-Jan-13 5:44am
   
Good catch on i, my 5.
nv3 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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
0 Sergey Alexandrovich Kryukov 838
1 OriginalGriff 461
2 Tadit Dash 355
3 sanket saxena 329
4 Peter Leow 213
0 Sergey Alexandrovich Kryukov 12,109
1 OriginalGriff 7,326
2 Peter Leow 5,003
3 Abhinav S 4,003
4 Maciej Los 3,575


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