TransWikia.com

Bash script for an ABC organization fulfilling the following requirements

Code Review Asked by usama on December 11, 2020

Write a bash script for an ABC organization fulfilling the following requirements:

  • Takes an Employee’s Name, designation, age, years of service and salary, extra working hours as an input.
  • Calculates remaining years of service. An Employee retires at the age of 60.
  • Check if the Employee is valid for salary appraisal. If an employee has been working for more than an year he deserves to be given an appraisal of 10%.
  • If an employee has given extra working hours within a month to the organization calculate bonus amount.
    Per hour amount varies depending upon the designation. See the given table to calculate bonus according to designations.
    Designation Per hour-amount
    Higher-Management 1K
    Lower-management 800PKR
    Other staff 500 PKR
  • Print the Employee’s information along with bonus and appraisal amount
read -p "Enter employee name: " ename # printing message and scanning variable

read -p "Enter designation: " designation # printing message and scanning variable

read -p "Enter age: " age # printing message and scanning variable

read -p "Enter years of service: " years # printing message and scanning variable

read -p "Enter salary: " salary # printing message and scanning variable

read -p "Enter extra working hours: " hours # printing message and scanning variable

years_remaining=$(( 60-age )) # calculating remaining years

#code to calculate appraisal

if [ "$years" -gt 1 ] # if condition

then

appraisal=$(( salary/10 )) # appraisal value

else

appraisal=0 # appraisal value

fi # end of if statement

#code to calculate the bonus

if [ "$designation" = "Higher-Management" ] # if condition

then

bonus=$(( 1000*hours )) # calculating bonus

elif [ "$designation" = "Lower-management" ] # else if condition

then

bonus=$(( 800*hours )) # calculating bonus

elif [ "$designation" = "Other staff" ] # else if condition

then

bonus=$(( 500*hours )) # calculating bonus

else

bonus=0

fi # end of if statement

echo "Employee name: $ename" # printing message

echo "Age: $age" # printing message

echo "Designation: $designation" # printing message

echo "Years of service: $years" # printing message

echo "Salary: $salary" # printing message

echo "Extra working hours: $hours" # printing message

echo "Bonus: $bonus" # printing message

echo "Appraisal: $appraisal" # printing message

echo "years_remaining:$years_remaining " # printing message

One Answer

Better comments

You have put a comment on almost every line that just restates what the code already says and does not add any information or clarification.

This is a bad practice!
A comment that states "this is an if statement" makes the code less readable, by doubling the length of a line with no added value.

When adding a comment, you should only do so if it provides information that is not in the code, or is difficult to extract from the code given average knowledge of the programing language.

Input validation

It is customary to check all user input before using it in your code.
Unless you have been explicitly instructed to assume that the input is valid, you should add some code, for example to make sure age is actually a number.

Also, you may want to check for cases where age >= 60 i.e. the employee is about to retire, or has retired already.

Better flow control

The long if elif else tree where you calculate the bonus should be replace with a case statement.
This will make the code shorter, more readable, and make adding or removing bonus options easier.

Also, store bonus multiplier in a variable to avoid repeating code:

case $designation in
    "Higher-Management")
        hour_bonus=1000
        ;;
    
    "Lower-management")
        hour_bonus=800
        ;;

    "Other staff")
        hour_bonus=500
        ;;

     *)
        hour_bonus=0
        ;;
esac

bonus=$((hours * hour_bonus))

Note, that you could also add shopt -s nocasematch before the case block to make the comparison case insensitive, so that if your user forgets to type capital letters (or forgets the caps lock on) your code can still match the designation to the proper bonus value.

Answered by Lev M. on December 11, 2020

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP