An annoying problem

[b][red]This message was edited by WerewolfWare at 2004-4-5 7:37:28[/red][/b][hr]
[b][red]This message was edited by WerewolfWare at 2004-4-5 7:35:42[/red][/b][hr]
I have an annoying problem.
I use TC-Lite in DOS and I need a function for string comparison.
This compiler's strcmp function compare 2 strings in length.
I built a function that will compare 2 strings and appearently it doesn't work.

This is the full file - it is a Simon game with number - preforms a sequance and the user must repeat it.
The lines marked in red are the function. Please tell me what is wrong?
If you spot another thing that might be the problem - let me know.

[code]
#include
#include
#include
#include
#include
#include
#include
#include
#include

#define VK_ENTER 13

#define SIMON_MAX 6


void RandSeq(char*,int); //Creates a random sequance of chars in the range of '1' to '4'
void GetString(char*,int); //Gets a string from the user
void DelayedOut(char*,int); //Preform an output of a string with a 0.8 seconds delay between everey char
void MemCpy(char*,char*,int); //Copies a certain number of chars from string "src" to string "dest"
int CompareStrings(char*,char*); //Compares 2 strings. If the strings are the same - returns 1. If the strings are diffrent - returns 0.


void main()
{

char* sequance = new char[ SIMON_MAX ]; //Stores the Simon sequance
char* user_seq; //Used to get a sequance from user
char* key_seq; //Used to store a corrent sequance of numbers

int simon_counter; //Counter for the loop

clrscr();

RandSeq(sequance, SIMON_MAX); //Randomize a sequance and store it in "sequance"

for( simon_counter = 0 ; simon_counter < SIMON_MAX ; simon_counter++ )
{

user_seq = key_seq = new char[ simon_counter +1 ];

MemCpy(key_seq, sequance, simon_counter +1); //Copies the corrent sequance of numbers

cout << "The sequance is:" << endl;
DelayedOut(key_seq, simon_counter +1);
delay(1700);

clrscr();

cout << "Insert sequance:" << endl;
//GetString(user_seq, simon_counter +1);
scanf("%s", user_seq);

clrscr();

if( CompareStrings(user_seq, key_seq) )
{
delete user_seq;
delete key_seq;
continue;

}

else if( !CompareStrings(user_seq, key_seq) )
{

delete user_seq;
delete key_seq;
delete sequance;

gotoxy((int)(strlen("#########You Lost!#########")/2), 12);
cprintf("#########You Lost!#########");
getch();
exit(1);
}

}

delete user_seq;
delete key_seq;
delete sequance;

gotoxy((int)(strlen("#########You Won!#########")/2), 12);
cprintf("#########You Won!#########");
getch();
exit(1);

}

void RandSeq(char *seq, int buff_sz)
{

int n;

srand( (unsigned) time(NULL) );

for( n = 0 ; n < buff_sz ; n++ )
seq[ n ] = ( rand() % 4 ) + '1';

seq[ buff_sz ] = NULL; //Don't notify me about this writing
//out of range

}

void GetString(char *in_stream, int buff_sz)
{

int n;
char c;

for( n = 0 ; n < buff_sz ; n++ )
{

//c = (char)getchar();

c = bioskey(0);

if( c == VK_ENTER )
{

++n;

cout << endl;

break;

}

if( c == ESC )
exit(1);

in_stream[ n ] = (char)c;

cout << c;
}

in_stream[ n ] = NULL; //Don't notify me about this writing
//out of range

if( n == buff_sz )
cout << endl;

}

void DelayedOut(char* string, int buff_sz)
{

int z;

for( z = 0 ; z < buff_sz ; z++ )
{

cout << string[ z ];
delay(800);

}

delay(300);
cout << endl;

}

void MemCpy(char* dest, char* src, int buff_sz)
{

int z;

for( z = 0 ; z < buff_sz && src[ z ] ; z++ )
dest[ z ] = src[ z ];

dest[ z ] = NULL; //Don't notify me about this writing
//out of range

}

[red]int CompareStrings(char *a, char *b)
{

if( strcmp( a, b ) * strcmp( a, b ) )
return 0;


int z;

for( z = 0 ; z < strlen( a ) && a[ z ] == b[ z ] ; z++ );

if( z != strlen( a ) )
return 0;

return 1;

}[/red]


[/code]
The lines marked in red are the function. Please tell me what is wrong?




Comments

  • [b][red]This message was edited by stober at 2004-4-5 8:0:39[/red][/b][hr]
    :
    : [red]int CompareStrings(char *a, char *b)
    : {
    :
    [blue]why are you comparing the strings twice??? strcmp() returns an int < 0, == 0 or > 0. Whatever it returns the first time will be the
    same as it returns the second time. So all you need is this:

    [green] return (strcmp(a,b) == 0) ? 1 : 0;[/green]

    And you can delete the rest of this function![/blue]
    : if( strcmp( a, b ) * strcmp( a, b ) )
    : return 0;
    :
    :
    : int z;
    :
    : for( z = 0 ; z < strlen( a ) && a[ z ] == b[ z ] ; z++ );
    :
    : if( z != strlen( a ) )
    : return 0;
    :
    : return 1;
    :
    : }[/red]
    :
    :
    : [/code]
    : The lines marked in red are the function. Please tell me what is wrong?
    :
    :
    :
    :
    :



  • [b][red]This message was edited by WerewolfWare at 2004-4-6 4:18:18[/red][/b][hr]
    Like I said befor - in my compiler - strcmp compares the strimgs's - my function compares content.
    What you wrote is useless to me - only on MS-VC strcmp compares the strings's content.


  • [b][red]This message was edited by Lundin at 2004-4-6 5:49:28[/red][/b][hr]
    : [b][red]This message was edited by WerewolfWare at 2004-4-6 4:18:18[/red][/b][hr]
    : Like I said befor - in my compiler - strcmp compares the strimgs's - my function compares content.
    : What you wrote is useless to me - only on MS-VC strcmp compares the strings's content.
    :
    :
    :


    strcmp() is an ANSI C function. If you compiler doesn't treat it the way stober described, then you should get a new compiler (even though I haven't seen a version of TC that implement strcmp in a different way).

    Anyway, if you want to write it yourself, a common way to implement it seems to be something like this:

    [code]
    int string_comp(const char* a, const char* b)
    {
    while (*a && *b)
    {
    if (*a != *b)
    return *a - *b;

    a++;
    b++;
    }
    return *a - *b;
    }
    [/code]



  • None of you is getting the concept.
    Look - the functions I gave earlier work's with a deiffrent main routin but for some reason this main routin doesnt work like I want it to.
    If I insert a bad string that doesn't match the current sequance it continue's instead of stopping - check my main function - tell me what's wrong and check the ComapreStrings function and see what I am trying to do.
  • : None of you is getting the concept.
    : Look - the functions I gave earlier work's with a deiffrent main routin but for some reason this main routin doesnt work like I want it to.
    : If I insert a bad string that doesn't match the current sequance it continue's instead of stopping - check my main function - tell me what's wrong and check the ComapreStrings function and see what I am trying to do.
    :

    [blue]2 comments:
    1. CompareStrings() will return unpredictable results if length of string b is less than length of string a. There is no check for that.


    3. main() does not need to call CompareStrings() twice -- if the first call returns 0 all you need is the else. You don't have to call it again to find out if it will return 0 because it just did that.
    [/blue]
  • [b][red]This message was edited by AsmGuru62 at 2004-4-6 11:28:26[/red][/b][hr]
    : None of you is getting the concept.
    : Look - the functions I gave earlier work's with a deiffrent main routin but for some reason this main routin doesnt work like I want it to.
    : If I insert a bad string that doesn't match the current sequance it continue's instead of stopping - check my main function - tell me what's wrong and check the ComapreStrings function and see what I am trying to do.
    :
    [blue]1. You code is littered with buffer overruns - and if you tell it "do not write me about this!" - it will not help. You talking to metal here! The code works just because the TC allows some rooms to be aligned and that is all. If you modify it somehow or move it to another compiler it will not work. So, instead of being arrogant you have to listen to people with experience. You want to be a professional coder - you have to understand C dogmas first: if you say "p=new char [6];" - you cannot do: "p[6]=...;" - it is a dogma. If you do not follow rules of C - your coding is doomed!

    2. Now, about your problem: so far I have seen this:
    [code]
    char* p1;
    char* p2;

    ...

    p1 = p2 = new char [...];
    [red]^^^ Not good!
    You allocate only ONE block here and not TWO!
    ---
    The solution here is this:
    p1 = new char [...];
    p2 = new char [...];
    [/red]

    ...

    delete p1;
    delete p2;
    [/code]
    3. Why using your own functions to perform string operations and memory operations - TC (even LITE) supposed to have them. The library functions are faster.

    4. Why not using an internal TC debugger?
    Does LITE have the debugger?
    If not, simply download TC 2.0 from ProgrammersHeaven.
    In debugger, you clearly see that pointers [b]p1[/b] and [b]p2[/b] point to the SAME area.

    5. Also, what is this supposed to do?
    [code]
    if (strcmp (...) * strcmp (...))
    {
    }
    [/code]
    Why the results from the function have to be multiplied?![/blue]


  • [b][red]This message was edited by stober at 2004-4-6 12:27:28[/red][/b][hr]
    :
    : 2. Now, about your problem: so far I have seen this:
    : [code]
    : char* p1;
    : char* p2;
    :
    : ...
    :
    : p1 = p2 = new char [...];
    : [red]^^^ Not good!
    : You allocate only ONE block here and not TWO!
    : ---
    : The solution here is this:
    : p1 = new char [...];
    : p2 = new char [...];
    : [/red]
    :
    : ...
    :
    [blue]He's also not allocating enough memory -- 1 byte too short (for
    null terminator). And delete should be delete[] [/blue]

    [/code]


  • [red]Reply to Stober's first message[/red]
    1. CompareStrings checks first if the length of the strings is ok - the same using strcmp().
    2.I call CompareStrings twice because at first it didn't work when I wrote:

    if( !Comparestrings(...) )
    {
    ........
    }

    so I added the else if() - I took it off my code after it didn't work.
    The reason I did it is because when I insert a wrong/right sequance it always continues the program.


    [red]Reply to AsmGuru[/red]
    1. Like I told you - don't write me about
    p = new char[6];
    p[6] =...;
    I didn't thought it worked too when I started programming - but appearantly it works and in many programs I had this was the problem - I didn't assign NULL to the last varible in an array.
    If you want to keep questioning this fact - go on but there is a reason I told you not to.

    2. Thanks for the comment about allocating memorey - I didn't know that but technichally I thought it supposed to work.

    3. The reason I did GetString() is because I couldn't think of another way to be able to get any key incuding ESC - none of the other functions can so I used bioskey(0).
    And CompareStrings() because strcmp() compares [b]length and not content[/b] and half of the people replyed to my message doesn't get this concept. My function compares content as well as length. The reason I wrote this message is not because CompareStrings() doesn't work - the program runs fine besides the part where it suppose to compare between the current sequance and the user input sequance.

    5. I wrote strcmp() * strcmp() because I wasn't shore if -1 (a possible returned value from strcmp) is a true expression.
    Squaring the result assures that the result will be either 1 or 0.


    [red]Reply to stober's second message[/red]
    1. It doesn't matter if the termination is preformed by delete or
    delete[] - most compilers are reffering to these commands the same.
    2. I allocated just enough memorey to the strings - current loop+1
    if loop is 0 that needs 0+1 space - if loop is 1 it needs 1+1 space etc'(...).


  • [b][red]This message was edited by WerewolfWare at 2004-4-7 3:44:32[/red][/b][hr]
    Thank you but I solved the problem:
    Appearently it was just as AsmGuru said - if I allocate memorey like this:

    p1 = p2 = new char[...];

    p1 refers to p2 therefor will never be diffrent from it because whe p2 will change so will p1 and vice versa...

    I just seporated the lines and the program became as perfect as it should be!

    Special thanks to AsmGuru for spotting the problem.
    [red]I need to mention that this was the only problem in the code - nothing else - all my functions and

    p1 = new char[6]
    p1[6] = NULL; // Only NULL is assignable here
    (I told you not to right about this )

    were right![/red]

    Don't be suprised I am writing this - I am 14 - I'm allowed to show-off once in a while :)


  • [b][red]This message was edited by stober at 2004-4-7 4:9:43[/red][/b][hr]
    : [red]I need to mention that this was the only problem in the code - nothing else - all my functions and
    :
    : p1 = new char[6]
    : p1[6] = NULL; // Only NULL is assignable here
    : (I told you not to right about this )
    :
    : were right![/red]

    [blue]No you are NOT right about that. All arrays are numbered from 0 up to, but not including, the number of elements in the array. So if you allocate the array of 6 bytes, as in your example above, the valid array indexes are:
    [code]
    p1[0]
    p1[1]
    p1[2]
    p1[3]
    p1[4]
    p1[5]
    [/code]

    When you address p1[6] you are addressing one element beyond the end of the array, somewhere in never-never land.

    Problem #2: scanf("%s") inserts a NULL terminating byte immediately after the characters are entered. So if you enter "Hello" the buffer needs to be at least 6 bytes (5 for the text and 1 for the null terminator). In your loop you are not allocating enough room for the null terminator.[/blue]


    :
    : Don't be suprised I am writing this - I am 14 - I'm allowed to show-off once in a while :)
    :
    [blue]you can show off when you have valid code. the code you posted is still full of errors even after making the change that AsmGuru pointed out to you..[/blue]
    :



  • : And CompareStrings() because strcmp() compares [b]length and not content[/b] and half of the people replyed to my message doesn't get this concept.

    No it doesn't. If you even bothered to read my earlier reply, you would have found some code that is identical to the ANSI C implementation of strcmp(). Compile the code and see how it compares the content as well as the length. And if strcmp() compares the length, then please tell me what strncmp() does :-)


    If you still don't believe me, here is the text from the standard:


    ISO 9899/1999:

    [italic]
    "7.21.4.2 The strcmp function

    Synopsis
    1 #include
    int strcmp(const char *s1, const char *s2);

    Description
    2 The strcmp function compares the string
    pointed to by s1 to the string pointed to by s2.

    Returns
    3 The strcmp function returns an integer greater than,
    equal to, or less than zero, accordingly as the string
    pointed to by s1 is greater than, equal to, or less than
    the string pointed to by s2."
    [/italic]
Sign In or Register to comment.

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Categories