0

I had to make a program wherein I was supposed to take age and experience(whether the employee is experienced or not) of an employee as input from the user and print out his/her salary. The salary was subject to the following conditions:

  • If employee is inexperienced, salary=2000 irrespective of age.
  • For an experienced employee:
    • If age<=28, salary=3000.
    • If 28<age<=35, salary=4800.
    • If age>35, salary=6000.

I made the following C++ program:

.
.
.
  cout<<"The salary of the employee is Rs."<<(experience?sal1:2000);  //LINE1
  sal1=((age<=28)?3000:sal2);
  sal2=((age<=35)?4800:6000);

  return 0;
}

where age, sal1, sal2 are declared as int and experience as bool.

experience=1 is entered by user for experienced employee and otherwise experience=0.

But whenever experience==1 and any age>28 is entered, I get unexpectedly large integral results whereas the code produces absolutely perfect results when conditional operator is nested.(i.e I copy the expression of sal1 to the truth expression in LINE1 and copy the expression of sal2 into expression of sal1)

Please explain what is the difference between both of these codes and why am I getting unexpected results in the first case.

NOTE: I have used the gcc g++ compiler for compilation of my code. Please tell if it's the fault of the compiler, the operator or is there any other issue.

t.niese
  • 39,256
  • 9
  • 74
  • 101
Nityunj
  • 11
  • 3
  • 5
    That code looks like it's written in reverse. You need to first calculate `sal2`, then `sal1`, and *then* you can print the value of `sal1`. (You must have some fundamental misunderstanding about what code means. I recommend a [good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and starting at chapter one.) – molbdnilo Nov 11 '20 at 08:18
  • 1
    Why you do so? Is really unreadable. Make well readable inline function or lambda. I can show you an example. – Nick Nov 11 '20 at 08:20
  • You have to calculate `sal2` before you can use its value to calculate `sal1`. Programs run fron the top to the bottom. – Galik Nov 11 '20 at 08:21
  • 1
    Turn on a higher warning level and look at the compiler warnings. The compiler will warn you that you are using an uninitialzed `sal1`/that you are reading `sal1` before writing to it. – Werner Henze Nov 11 '20 at 08:23
  • You can't print the result before you have calculated it. Not in an imperative language like `C++`. – Galik Nov 11 '20 at 08:23
  • Unrelated to your problem, but when every you ask questions here use the [edit] functionality and the preview to improve the formatting of your question. Especially splitting a larger chunk of text into multiple blocks improves readability. And providing a [mcve] instead of writing `where age, sal1, sal2 are declared as int and experience as bool.` is also helpful. – t.niese Nov 11 '20 at 08:29
  • Thanks a lot!!!@molbdnilo and galik That worked just perfectly!!!! Thank you for the suggestions @t.niese and Nick. I'll try to improve that from next time onwards – Nityunj Nov 11 '20 at 08:36

3 Answers3

0

I think it would be easier to understand the logic, both for writing and for reading, if you used plain if ... else if ... else instead.

Perhaps something like

int salary;
if (age <= 28)
{
    salary = 3000;
}
else if (age > 28 && age <= 35)
{
    salary = 4800;
}
else
{
    // Age must be over 35 to come here
    salary = 6000;
}
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 2
    Looking at the error that the questioner made in his code (using result before calculating it) I doubt that your answer helps him. – Werner Henze Nov 11 '20 at 08:30
0

No need such a difficult to read (and write) code:

#include <iostream>

int calcSalary(int age, bool experience){
   if (!experience)
      return 2000;

   if (age <= 28)
      return 3000;

   if (age <= 35)
      return 4800;

   return 6000;
}

int main(){
   std::cout << "The salary of the employee is Rs." << calcSalary(36, false) << '\n';

   return 0;
}

Assuming C++11, you can use lambda:

#include <iostream>
    
int main(){
   auto calcSalary = [](int age, bool experience){
      if (!experience)
         return 2000;

      if (age <= 28)
         return 3000;

      if (age <= 35)
         return 4800;

      return 6000;
   };

   std::cout << "The salary of the employee is Rs." << calcSalary(36, false) << '\n';

   // no return 0 need for C++11
}
Nick
  • 9,962
  • 4
  • 42
  • 80
  • But why `inline` and why using a lambda here? Just because you CAN use something doesn't mean it's the appropriate thing to use. And you missed that there were 2 arguments instead of one. The comment about the unnecessary return is misleading for a beginner, it's just not needed for the main. – Stefan Riedel Nov 11 '20 at 08:36
  • Historically `inline` means "inline the function". But nowadays `inline` helps defining the function in header file - It is not currently there, but it may be moved. Same for `static` and anonymous namespace. However anonymous namespace is bit over the top for such a small example. – Nick Nov 11 '20 at 08:39
  • There is no need for the second argument.... I think... Is very unreadable. My function do exactly what the description say. – Nick Nov 11 '20 at 08:41
  • Right, but as you said, it's in the main.cpp. So it does effectively nothing here and just adds noise for a beginner – Stefan Riedel Nov 11 '20 at 08:41
  • No it does not. The OP said the salary is 2000 if the employee is inexperienced – Stefan Riedel Nov 11 '20 at 08:42
  • in main.cpp `inline` do something. It prevent creating the function in the object file. If you do without, both "calcSalary" and "main" will be "exposed" in the object file. – Nick Nov 11 '20 at 08:43
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/224403/discussion-between-stefan-riedel-and-nick). – Stefan Riedel Nov 11 '20 at 08:54
  • `// no return 0 need for C++11` is not just confusing, strictly speaking it is wrong. The implicit return from main has nothing to do with C++11 – 463035818_is_not_an_ai Nov 11 '20 at 09:17
  • It will be more confusing if I omit this comment. – Nick Nov 11 '20 at 11:18
0

If you really want to use nested ternary operators (for instance if you want to use const everywhere), then you can do something like this:

auto const salary = (!isExperienced) ? 2000 :
                    (age <= 28 ) ? 3000 :
                    (age <= 35 ) ? 4800 : 6000;

remember that the ternary (?) operator works like this:

(conditional-statement) ? (if-true) : (if-false)

when you chain them together like in the example, then they are evaluated sequentially from top to bottom. So testing for isExperience is first, and if the employee is NOT experienced then we evaluate as 2000. From there we test if less than or equal to 28, less than or equal to 35.

Alex Shirley
  • 385
  • 4
  • 11