2

Code, as shown below, can be complied.

Problem: It always says "Invalid Employee id" even when I enter the correct employee id.
Please tell me why and how to do this correctly.

#include <iostream>
#include <iomanip>
using namespace std;
char select, js;
char empid[4];
double bSalary, bonus, tot=0.0;
int main()
{
    do
    {
        cout<<"Employee id: ";
        cin>>empid;
        if(empid=="M001" || empid=="A004" || empid == "M002") //these are employee ids
        {   
            cout<<"Job Status: ";
            cin>>js;
            if(js=='P' || js=='C')
            {
                cout<<"Basic Salary: ";
                cin>>bSalary;
                if(bSalary>75000 && js=='P')
                {
                    bonus = bSalary*(20.0/100.0);
                    tot = tot + bonus + bSalary;
                }
                else if(bSalary>75000 && js=='C')
                {
                    bonus = bSalary*(15.0/100.0);
                    tot = tot + bonus + bSalary;
                }
                else
                    tot = tot+bonus+bSalary;
            }
            else
                cout<<"Invalid Job Status"<<endl;
        }
        else
            cout<<"Invalid Employee no"<<endl;
        cout<<"Do you want to continue: ";
        cin>>select;
        cout<<endl;
    }while(select=='y'||select=='Y');
    cout<<"Total cost: "<<setprecision(2)<<setiosflags(ios::fixed)<<tot<<endl;
    return 0;
}

Note: It is going to the else clause all the time.

Improve this question
2
  • 5
    == applied to arrays compares pointers (the addresses of the arrays), which you don't want. Use std::string instead of raw arrays. std::string supports comparisons via ==. Commented Jun 9, 2015 at 3:07
  • 2
    You cannot compare C-style strings with the == operator. Either use strcmp() or switch to C++ strings. Commented Jun 9, 2015 at 3:08

4 Answers 4

Reset to default
11

It's this:

char empid[4];

This is too small as there's no room for a NUL terminator after the id. You could just change it to 5, but then if someone deliberately or accidentally typed a longer string your program may crash (it's called a buffer overrun, and in some situations can allow whoever provides input to hack the account running the program).

Further, == doesn't work for character arrays: you have to use e.g.:

 if (strcmp(empid, "M001") == 0 || strcmp(empid, "A004") == 0 || ...

You would be much better off using a std::string, which will grow to accommodate the actual input (including a NUL terminator though that's not counted in a string's .size()), and works intuitively with ==.

Separately, your...

tot = tot+bonus+bSalary;

...is broken, as bonus may be uninitialised you mustn't read from it in an expression. You can simply remove bonus from the addition above, if it's meant to be 0 for the relevant employees.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

how to use std::string, when i use std::string instead of char it gets an error. Ex std::string empid;
@Nishan256 you'll need to put #include <string> atop the file too.
4

You can't compare C strings with == or !=. As you can see that empid here is just a pointer so == will compare the base addresses of those strings not the strings themselves.

You need to use strcmp and include 

#include <cstring>
.
.
.
if(strcmp(empid,"M001")==0 || ...)
Improve this answer

Comments

3

Empid is not a string. C++ doesn't have a built-in == overload to compare char arrays.

You can make your own == operator. But don't bother. Declare empid as a string and watch magic happen.

string empid;
Improve this answer

Comments

2

Changing the size of the char array to take care of NULL char will not work here.

char empid[5]

"==" operator do not work properly with char arrays. please change the condition to below:

if (0 == (strcmp(empid, "M001")) || (0 == (strcmp(empid, "A004"))) || (0 ==     
   (strcmp(empid, "M002"))))

EDIT:people above has already answered your question. My answer is redundant now.

Improve this answer

5 Comments

what we expect from 0== ?
@Nishan256: when both strings match then strcmp returns 0. "0 == " is a part of a coding guideline to avoid assignment errors. for example, if we have a condition if(i == 0). Now by mistake we make it if(i =0). This means we are assigning zero to i and compiler will never give a error. Change this to if(0 ==1). Now if we make a mistake and write if(0 = i), this would be caught by the compiler. Thus this makes the code logic error proof and is a good habit to follow. many coding guidelines found over internet also suggest this.
@Nishan256 strcmp returns 0 when given equal strings. A larger string returns a positive number and a smaller string returns a negative number.
Just for the sake of providing the flip side: most compilers support warnings for e.g. if (i = 0), removing the danger of accidental use, but there is some pain: to tell the compiler "no, I really do want to do that" you may have to denote that by surrounding the assignment with extra parenthesis ala if ((i = 0)) (well, it makes more sense for non-literal assignments!).
@TonyD : Thanks for the Update!!! I have made similar errors leading to lot of rework :) So nowadays I am careful. Plus when you are working on a project with thousands of file, you generally skip over non critical warning. that what cost me a lot of time when I made a mistake. So now I got myself into this habit.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.