Implement Review Suggestion

<UserStory>EMP008: Update Salary Details</UserStory>

    <Changes>
  - Refactored salary update and employee selection:
  • Removed unnecessary const qualifiers from double parameters
  • Renamed variables
  • Replaced count() with find() for map lookups
  • Excluded ADMIN employees from salary updates and selection lists
  • Applied util::enforceAuthorization for access control
- Improved readability and consistency:
  • Enhanced tabular output using setw() with left alignment
    Implement Update Salary
    </Changes>

    <Review>
    Smitha Mohan
    </Review>
This commit is contained in:
Jissin Sam Mathew
2026-04-07 20:19:27 +05:30
committed by Joel Thomas
parent 63627075ef
commit 0290467528
7 changed files with 47 additions and 35 deletions
@@ -32,7 +32,7 @@ void ZenvyController::updateProfile(const std::string& name, const std::string&
m_employeeManagementService->updateProfile(name,phone); m_employeeManagementService->updateProfile(name,phone);
} }
void ZenvyController::updateSalary(const std::string& employeeId, const double basicSalary, const double houseRentAllowance, const double foodAllowance, const double employeePFContribution, const double employerPFContribution) void ZenvyController::updateSalary(const std::string& employeeId, double basicSalary, double houseRentAllowance, double foodAllowance, double employeePFContribution, double employerPFContribution)
{ {
m_payslipManagementService->updateSalary(employeeId, basicSalary, houseRentAllowance, foodAllowance, employeePFContribution, employerPFContribution); m_payslipManagementService->updateSalary(employeeId, basicSalary, houseRentAllowance, foodAllowance, employeePFContribution, employerPFContribution);
} }
@@ -50,3 +50,7 @@ Employees ZenvyController::getEmployees()
{ {
return m_employeeManagementService->getEmployees(); return m_employeeManagementService->getEmployees();
} }
std::shared_ptr<const Employee> ZenvyController::getEmployee(const std::string&) {
}
@@ -52,6 +52,6 @@ public:
std::shared_ptr<const Employee> getCurrentEmployee(); std::shared_ptr<const Employee> getCurrentEmployee();
void updateProfile(const std::string&,const std::string&); void updateProfile(const std::string&,const std::string&);
//payslip management //Payslip management
void updateSalary(const std::string&, const double, const double, const double, const double, const double); void updateSalary(const std::string&, double, double, double, double, double);
}; };
@@ -14,7 +14,6 @@ public:
EmployeeManagementService() : m_dataStore(DataStore::getInstance()) {}; EmployeeManagementService() : m_dataStore(DataStore::getInstance()) {};
void createEmployee(Enums::EmployeeType, Enums::EmployeeDesignation, const std::string&, const std::string&, const std::string&); void createEmployee(Enums::EmployeeType, Enums::EmployeeDesignation, const std::string&, const std::string&, const std::string&);
bool deactivateEmployee(const std::string&); bool deactivateEmployee(const std::string&);
Employees getEmployees(); Employees getEmployees();
std::shared_ptr<const Employee> getEmployee(const std::string&); std::shared_ptr<const Employee> getEmployee(const std::string&);
void updateProfile(const std::string&,const std::string&); void updateProfile(const std::string&,const std::string&);
@@ -1,24 +1,21 @@
#include "PayslipManagementService.h" #include "PayslipManagementService.h"
#include "AuthorizationHelper.h"
#include "Enums.h"
void PayslipManagementService::updateSalary(const std::string& employeeId, const double basicSalary, const double houseRentAllowance, const double foodAllowance, const double employeePFContribution, const double employerPFContribution) void PayslipManagementService::updateSalary(const std::string& employeeId, double basicSalary, double houseRentAllowance, double foodAllowance, double employeePFContribution, double employerPFContribution)
{ {
if (m_dataStore.getAuthenticatedEmployee()->getEmployeeType() == Enums::EmployeeType::FINANCE) { util::enforceAuthorization(m_dataStore.getAuthenticatedEmployee()->getEmployeeType(), Enums::EmployeeType::FINANCE);
auto findedEmployee = m_dataStore.getEmployees().find(employeeId); auto employee = m_dataStore.getEmployees().find(employeeId);
if (findedEmployee != m_dataStore.getEmployees().end()) if (employee != m_dataStore.getEmployees().end() && employee->second->getEmployeeType() != Enums::EmployeeType::ADMIN)
{ {
(findedEmployee->second)->getPayroll()->setBasicSalary(basicSalary); (employee->second)->getPayroll()->setBasicSalary(basicSalary);
(findedEmployee->second)->getPayroll()->setHouseRentAllowance(houseRentAllowance); (employee->second)->getPayroll()->setHouseRentAllowance(houseRentAllowance);
(findedEmployee->second)->getPayroll()->setFoodAllowance(foodAllowance); (employee->second)->getPayroll()->setFoodAllowance(foodAllowance);
(findedEmployee->second)->getPayroll()->setEmployeePFContribution(employeePFContribution); (employee->second)->getPayroll()->setEmployeePFContribution(employeePFContribution);
(findedEmployee->second)->getPayroll()->setEmployerPFContribution(employerPFContribution); (employee->second)->getPayroll()->setEmployerPFContribution(employerPFContribution);
}
else
{
throw std::runtime_error("Employee not found, unable to update the salary");
}
} }
else else
{ {
throw std::runtime_error("Unauthorized access"); throw std::runtime_error("Employee not found, unable to update the salary");
} }
} }
@@ -9,5 +9,5 @@ private:
DataStore& m_dataStore; DataStore& m_dataStore;
public: public:
PayslipManagementService() : m_dataStore(DataStore::getInstance()) {}; PayslipManagementService() : m_dataStore(DataStore::getInstance()) {};
void updateSalary(const std::string&, const double, const double, const double, const double, const double); void updateSalary(const std::string&, double, double, double, double, double);
}; };
@@ -31,28 +31,39 @@ void FinanceExecutiveMenu::run()
std::string FinanceExecutiveMenu::getSelectedUserId() std::string FinanceExecutiveMenu::getSelectedUserId()
{ {
int choice; int choice;
std::map<int, std::shared_ptr<const Employee>> currentEmployeeList; std::map<int, std::shared_ptr<const Employee>> employeeList;
int index = 0; int index = 0;
for (auto& currentEmployee : m_zenvyController->getEmployees()) auto allEmployees = m_zenvyController->getEmployees();
for (auto& currentEmployee : allEmployees)
{ {
currentEmployeeList[++index] = currentEmployee; if (currentEmployee->getEmployeeType() == Enums::EmployeeType::ADMIN)
{
continue;
}
employeeList[++index] = currentEmployee;
} }
for (auto& currentEmployee: currentEmployeeList) std::cout << std::left
<< std::setw(6) << "Index"
<< std::setw(15) << "Employee Id"
<< std::setw(25) << "Name" << std::endl;
for (const auto& employee : employeeList)
{ {
std::cout << currentEmployee.first << ". Employee Id: " << currentEmployee.second->getEmployeeId() << "Name: " << currentEmployee.second->getEmployeeName() << std::endl; std::cout << std::left << std::setw(6) << employee.first
<< std::setw(15) << employee.second->getEmployeeId()
<< std::setw(25) << employee.second->getEmployeeName()
<< std::endl;
} }
std::cout << "Enter the Index: "; std::cout << "Enter the Index: ";
util::read(choice); util::read(choice);
if (currentEmployeeList.count(choice) == 0) auto employeeIterator = employeeList.find(choice);
if (employeeIterator != employeeList.end())
{ {
throw std::runtime_error("Enter a valid index"); return (employeeIterator->second->getEmployeeId());
} }
else else
{ {
auto selectedEmployee = currentEmployeeList.find(choice); throw std::runtime_error("Invalid Index");
return (selectedEmployee->second->getEmployeeId());
} }
} }
void FinanceExecutiveMenu::updatePayroll() void FinanceExecutiveMenu::updatePayroll()
@@ -78,7 +89,6 @@ void FinanceExecutiveMenu::updatePayroll()
} }
} }
bool FinanceExecutiveMenu::handleOperation(int choice) bool FinanceExecutiveMenu::handleOperation(int choice)
{ {
switch (choice) switch (choice)
@@ -1,6 +1,8 @@
#pragma once #pragma once
#include<memory> #include<memory>
#include<iostream>
#include<stdexcept> #include<stdexcept>
#include <iomanip>
#include"ZenvyController.h" #include"ZenvyController.h"
class FinanceExecutiveMenu class FinanceExecutiveMenu