My intro to control structures

[Chef]

I have come across a small problem that I have been unable to tinker out.


At the very end of the code, the last question that is output, the answer no matter what number i put says "That value is positive". Even if I enter 0.

Code: [Select]


#include <iostream>
#include <string>
#include <sstream>
#include <conio.h>
using namespace std;


int main ()
{
    string mystr;
    string mystr2;
    string mystr3;
    float price=0;
    int quantity=0;
    float data=0;
    float data2=0;




    cout << "Hello, please enter a price: ";
    getline (cin,mystr);
    stringstream(mystr) >> price;
    cout << "Thank you. Please enter a quantity: ";
    getline (cin,mystr);
    stringstream(mystr) >> quantity;
    cout << "Your final price is: " << price*quantity << endl;
    cout << "Please enter another integer: ";
    getline (cin,mystr2);
    stringstream(mystr2) >> data;
    {
        if (data > 0)
  cout << "That value is positive.";
else if (data < 0)
  cout << "That value is negative.";
else
  cout << "That value is 0.";
    }
    cout << "Thank you for your time." << endl << "Enter one last integer to fully accomadate the engineering in this program:";
    getline (cin,mystr3);
    stringstream(mystr3) >> data2;
    {
        if (data > 0)
            cout << "That value is positive.";
            else if (data < 0)
            cout << "That value is negative.";
        else
            cout << "That value is zero.";
    }




  _getch();
  return 0;
}


[theifyppl]

Shouldn't your if statements be checking data2, not data? In the last part I mean. Sorry if I'm mistaken, I didn't read through the whole thing because I'm not home right now.

[Chef]

You are correct. I can't believe I missed that obvious error! Thanks!




Below I have a code that is executing fine and there are no build errors.
However, when I answer "black" or "white" it only output's the first optional response I have which is "Did you know black is a mixture of all colors?"


*Ignore the dumb questions in the code, it's just fillers for me to test with.

Code: [Select]

#include <iostream>
#include <string>
#include <sstream>
#include <conio.h>
using namespace std;


int main ()
{
    string mychar;
    char black;
    char Black;
    char white;
    char White;
    cout << "Do you prefer black or white?";
    cin >> mychar;
    {
    if (mychar = black, Black)
        cout << "Did you know black is a mixture of all colors?";
    else if (mychar = white, White)
        cout << "White is nice looking.";
    else
        cout << " That is not a valid response.";
   }
    _getch();
    return 0;


}


[Fur]

Code: [Select]

    if (mychar = black, Black)
        cout << "Did you know black is a mixture of all colors?";
I think there's your problem.
First of all, you're assigning black to myChar.

This will return true since that would return the new value of myChar and cast it to bool, which will always be true.

Regarding the comma operator in C++:

the comma operator (represented by the token ,) is a binary operator that evaluates its first operand and discards the result, and then evaluates the second operand and returns this value (and type).

In this case, it functions as an "Ok, fuck that last statement. [the second operand] is going to be returned".
Since a char cast to bool is always true (1), we're just doing an if (true) statement (do they have a name?).

So what you're basically doing:
Assigning the value of Black to mychar in an if statement that is always going yield true.

To compare stuff we use "==". If you're trying to check whether mychar is equal to black AND Black, do something like this: "if (mychar == black && mychar == Black) {".

Don't know if this is the problem you're having, but this is an issue anyway.

P.S: I know next to nothing about C++ (though I do know a little bit of C).
P.S.S: Your code is truly horrible (no offence).

I've improved (more like rewritten but nevermind) your code.
Now it accepts any case, and doesn't throw a hissy fit when the user enters some trailing/leading spaces.
Here's my fix (although I've had to improvise a little):

Code: (Cpp) [Select]

using namespace std;
#include <iostream>
#include <string>
#include <algorithm> // For the uppercase function. Maybe including an entire file for a single function is a bit overkill.

int main(int argc, char** argv) {
    string preferredColour;
    cout << "Do you prefer black or white? ";
    cin >> preferredColour;

    std::transform(preferredColour.begin(), preferredColour.end(),
        preferredColour.begin(), ::toupper); // Convert the input to uppercase.

    if (preferredColour.find("BLACK") != -1) {
        cout << "Did you know black is a mixture of all colors?";
    } else if (preferredColour.find("WHITE") != -1) {
        cout << "White is nice looking.";
    } else {
        cout << " That is not a valid response.";
    }
    // _getch();
    return 0;
}


[Chef]

That does look alot better. I'm not being lazy and just writing shitty code - this is code I have written from what I been learning from the tutorials, so my bad.
Thanks for answering my questions.

Below is a code I'm working on and I have these 3 error's that I cannot find the correction to.


The error's are here:


error: 'option' was not declared in this scope (line 23)
error: expected ')' before numeric constant (line 23)
error: expected ')' before numeric constant (line 36)

Code: [Select]

using namespace std;
#include <iostream>
#include <sstream>
#include <conio.h>
#include <string>
#include <windows.h>
int main ()
{
    string mystr;
    string mystr2;


    float price=0;
    int quantity=0;
    unsigned char x;


    cout << "Please select an option." << endl;


    cout << "For price and quantity calc, enter: option 1" << endl;


    cout << "For today's date, enter: option 2";
    cin >> x;


    if (x= option 1)
    {
        cout << "Please enter a price:";
        getline (cin, mystr);
        stringstream(mystr) >> price;


        cout << "Please enter a quantity:";
        getline (cin, mystr2);
        stringstream(mystr2) >> quantity;


        cout << "The final price is, " << price*quantity << ".\n";
    }


    else (x= option 2 )
    {
        GetSystemTime(p_st);


cout << "Year: " << p_st->wYear << endl;


cout << "Month: " << p_st->wMonth << endl;


cout << "Day: " << p_st->wDate << endl;


cout << "Hour: " << p_st->wHour << endl;


cout << "Minutes: " << p_st->wMinute << endl;


cout << " Seconds: " << p_st->wSeconds << endl;


cout << "Milliseconds: " << p_st->wMilliseconds << endl;
    };






    _getch();
    return 0;
}


[theifyppl]

That does look alot better. I'm not being lazy and just writing shitty code - this is code I have written from what I been learning from the tutorials, so my bad.
Thanks for answering my questions.

Don't feel bad at all.  You're still learning and it takes time to be able to write clean, well organized code.  The errors you have below are because of your if statements.

if (x= option 1) does not work for a number of reasons.  First of all, in any if statement, you need a boolean operation (true/false outcome) to determine if the if statement should run.  The boolean operator to check if something is equivalent is "==" not "=".  This is very important.  Always use "=" when assigning a value to a variable, and always use "==" when comparing values.  This is what Fur was saying as well.

So now we have if (x == option 1) so far.

Second of all, I don't understand what you're trying to compare x to.  Are you seeing if x equals the string "option 1"?  If so, there's two things that need to be addressed: if the user enters "hi my name is james" and you use "cin," cin will only return the first word ("hi").  Therefore, in this case, you should use a getline() function.  Secondly, you would need quotation marks around the "option 1" in your if statement:

if (x == "option 1")


Note: Actually, if you're comparing strings, you should use the strcmp() function.  There's a good example of how to use it in the third post here: http://www.cplusplus.com/forum/beginner/38544/
However, if (x == "option 1") will probably still work.

EDIT: Nevermind, strcmp() is for C strings, not C++ strings. Thank Fur for pointing that out.

Lastly, if you were intending to compare x to a variable "option 1," know that variable names are not allows to have spaces in them.  So you would need to declare a variable named option1, or something.  But I assumed you were comparing x to the string "option 1."

Everything I just mentioned also applies to line 36, which I assume is where you have if (x == option 2). 

Edit: I also see that you have x declared as an unsigned char.  Were you intending the user to just enter 1 or 2? If so, you should be more clear in your instructions.  And your if statement would look like:

if (x == '1') or if (x == '2')

Please note the single quotes instead of doubles quotes.  Using double quotes, like "1," tells the compiler that you're comparing to an array of characters (a string) that only contains one character.  Using single quotes, like '1', tells the compiler that you're comparing to the character '1'.

[Fur]

Note: Actually, if you're comparing strings, you should use the strcmp() function.

Isn't that primarily a C function? If so, I don't think it's a good idea to mix-and-match C and C++.

Anyway:

In C++ the std::string class implements the comparison operators, so you can perform the comparison using == just as you would expect

Source: http://stackoverflow.com/questions/6222583

Also, you're a lot better at explaining things than me, Theifyppl

[theifyppl]

Isn't that primarily a C function? If so, I don't think it's a good idea to mix-and-match C and C++.

Anyway:Source: http://stackoverflow.com/questions/6222583

Also, you're a lot better at explaining things than me, Theifyppl

Ah you're totally right. I was getting my C strings and C++ Strings mixed up. I just remembered having to use the strcmp() function before, but that's when I was using C strings with sockets. Good save +1

And thank you, haha. You're definitely not bad yourself, though.