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
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.
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.
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
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP