Howdy, Stranger!

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

Categories

Whats wrong with this?

[code]#include
#include
#include
using namespace std;

int choice = 1;
int thing;
int main(int argc, char *argv[])
{
for (; choice = 1 ;)
{
ifstream load("lol.lol");
load >> thing;
cout << thing << "
Would you like to change the data?
Yes=1
No=2" << endl;
cin >> choice;
if (choice = 1)
{
cout << "Type the new data" << endl;
cin >> thing;
ofstream save("lol.lol");
save << thing;
}
else if (choice = 2)
{
system("PAUSE");
}
}
}

[/code]

Anything wrong with it?

Comments

  • MT2002MT2002 Member Posts: 1,444
    Assuming this is a generic question, yes.

    1) When you load the file, you use it without ever verifying that it is actually opened;

    2) Using assignment (=) instead of test for equality (==) operators;

    3) [b]for (; choice = 1 ;)[/b] will result in an infinity loop and [b]if (choice = 1)[/b] will always be true (Both do to the above error)

    There are several other nit picky details that I personally do not recommend in the code. If you want, I can post those too. Nonetheless they are not as bad as the above three errors.

    *edit: Oh wait, one more:

    4) No formatting. I consider this an error and not a nit pick, so decided to add it here as well. Proper formatting is very important.
  • davidthefatdavidthefat Member Posts: 3
    : Assuming this is a generic question, yes.
    :
    : 1) When you load the file, you use it without ever verifying that it
    : is actually opened;
    :
    : 2) Using assignment (=) instead of test for equality (==) operators;
    :
    : 3) [b]for (; choice = 1 ;)[/b] will result in an infinity loop and
    : [b]if (choice = 1)[/b] will always be true (Both do to the above
    : error)
    :
    : There are several other nit picky details that I personally do not
    : recommend in the code. If you want, I can post those too.
    : Nonetheless they are not as bad as the above three errors.
    :
    : *edit: Oh wait, one more:
    :
    : 4) No formatting. I consider this an error and not a nit pick, so
    : decided to add it here as well. Proper formatting is very important.
    :
    I dont quite understand #2,



    [code]#include
    #include
    #include
    using namespace std;

    int choice;
    int thing;
    int main(int argc, char *argv[])
    {
    cout << "
    Would you like to change the data?
    Yes=1
    No=2" << endl;
    cin >> choice;
    if (choice == 1)
    {
    ifstream load("lol.lol");
    load >> thing;
    cout << thing << endl;


    cout << "Type the new data" << endl;
    cin >> thing;
    ofstream save("lol.lol");
    save << thing;
    }
    else if (choice == 2)
    {
    system("PAUSE");
    }
    }


    [/code]
  • LundinLundin Member Posts: 3,711
    : I dont quite understand #2,

    This is the equality operator:

    if (x == y)

    It checks if x is equal to y.


    This is the assignement operator:

    x = y;

    It copies the value of y to x.

    if (x = y)

    copies the value of y to x, then checks if x is larger than 0.
  • davidthefatdavidthefat Member Posts: 3
    : : I dont quite understand #2,
    :
    : This is the equality operator:
    :
    : if (x == y)
    :
    : It checks if x is equal to y.
    :
    :
    : This is the assignement operator:
    :
    : x = y;
    :
    : It copies the value of y to x.
    :
    : if (x = y)
    :
    : copies the value of y to x, then checks if x is larger than 0.
    [code]#include
    #include
    #include
    using namespace std;

    int choice = 1;
    int thing;
    int main(int argc, char *argv[])
    {
    for (; choice == 1 ;)
    {
    ifstream load("lol.lol");
    load >> thing;
    cout << thing << "
    Would you like to change the data?
    Yes=1
    No=2" << endl;
    cin >> choice;
    if (choice == 1)
    {
    cout << "Type the new data" << endl;
    cin >> thing;
    ofstream save("lol.lol");
    save << thing;
    }
    else if (choice == 2)
    {
    system("PAUSE");
    }
    }
    }
    [/code]


    That better? it runs as I want it too
  • MT2002MT2002 Member Posts: 1,444
    : That better? it runs as I want it too

    > Change [b]for (; choice == 1 ;)[/b] to [b]while (choice==1)[/b]. Not only is it easier to read but demonstrates your intentions.

    There are alot of other flaws in the code itself from a design perspective; but I think Ill just stop here :-)

Sign In or Register to comment.