I asked this question initially on CodeReview. Because of negative reception there (completely my fault), I think this would be the right place to ask. I am copying the exact same question here that I asked there (with some minor edits).
My goal is to write a CPU Scheduling Simulator in C++ using the STL features as far as possible. So far, I have only written the code for FCFS and have not provided any input method.
What sticks out most to me is the definition of the process class. The way I have designed the program, there is very poor encapsulation. Looking at my current situation, I have only two solutions in mind:
Make all data members private. Provide accessors for PID, Arrival Time and Burst Time and mutators for the rest. However, I fear this would make my code bloated and also the use of mutators would only break encapsulation.
Make FCFS(), displayResult() and any other algorithms that I add friends of the process class. This allows me to control the encapsulation to a certain extent so I like this one.
Another thing I tried to do was make the PID, Arrival Time and Burst Time variables const and public. This allowed algorithms to read their values but not modify them. However, the code wouldn't compile because const members aren't default assignable.
I would like suggestions as to how I can solve the above mentioned problem and also how I can use the STL more effectively to make my code more expressive.
#include <iostream>
#include <vector>
#include <algorithm>
#include <limits>
constexpr int notYetComputed = std::numeric_limits<unsigned int>::max();
struct process
{
unsigned int m_procId;
unsigned int m_arrivalTime;
unsigned int m_burstTime;
unsigned int m_responseTime;
unsigned int m_completionTime;
unsigned int m_turnaroundTime;
unsigned int m_waitingTime;
process(unsigned int pid, unsigned int at, unsigned int bt)
: m_procId {pid}, m_arrivalTime {at}, m_burstTime {bt}
{
m_waitingTime = m_turnaroundTime = m_completionTime = m_responseTime = notYetComputed;
}
};
void displayResult(const std::vector<process>& procs)
{
for(auto& x : procs)
std::cout << "PID: " << x.m_procId << ", "
<< "Waiting Time: " << x.m_waitingTime << ", "
<< "Turnaround Time: " << x.m_turnaroundTime << ", "
<< "Response Time: " << x.m_responseTime << ", "
<< "Completion Time: " << x.m_completionTime << "\n";
}
void FCFS(std::vector<process>& procList)
{
//Sort based on arrival order. Use PID in case of same arrival time.
auto arrivalOrder = [] (const process& p1, const process& p2) {
if(p1.m_arrivalTime < p2.m_arrivalTime) return true;
if(p1.m_arrivalTime == p2.m_arrivalTime) return (p1.m_procId < p2.m_procId);
return false;
};
std::sort(procList.begin(), procList.end(), arrivalOrder);
unsigned int clock {0};
auto computeResult = [&clock] (process& pr) {
pr.m_responseTime = clock - pr.m_arrivalTime;
pr.m_waitingTime = (pr.m_turnaroundTime =
(pr.m_completionTime = (clock += pr.m_burstTime)) - pr.m_arrivalTime)
- pr.m_burstTime;
};
std::for_each(procList.begin(), procList.end(), computeResult);
}
int main()
{
std::vector<process> procs {{0,0,5}, {1,1,3}, {2,2,8}, {3,3,6}};
FCFS(procs);
//Sort based on PID before showing result
std::sort(procs.begin(), procs.end(),
[](const process& p1, const process& p2) {
return p1.m_procId < p2.m_procId;
});
displayResult(procs);
}
UPDATE:
I intend to add more scheduling algorithms in the future. So I prefer answers along Jarod42's line of thought. I can't really make algorithm-specific code part of the process class.
Your process class contains both input and output members.
I would split them (Names I use should probably be improved) to be explicit about the flow (and we can get rid of notYetComputed which seems to be an hack for me):
struct process_input
{
unsigned int m_procId;
unsigned int m_arrivalTime;
unsigned int m_burstTime;
};
struct process_ouput
{
unsigned int m_responseTime;
unsigned int m_completionTime;
unsigned int m_turnaroundTime;
unsigned int m_waitingTime;
};
struct process
{
process_input m_input;
process_output m_output;
};
FCFS would show the flow, so std::vector<process> FCFS(std::vector<process_input> inputs)
With some style changes, it becomes:
std::vector<process> FCFS(std::vector<process_input> inputs)
{
// Sort based on arrival order. Use PID in case of same arrival time.
auto proj = [](const process_input& p){ return std::tie(p.m_arrivalTime, p.m_procId); };
auto compare = [&] (const process& lhs, const process& rhs) {
return proj(lhs) < proj(rhs);
};
std::sort(inputs.begin(), inputs.end(), compare);
std::vector<process> res;
res.reserve(inputs.size());
unsigned int clock {0};
auto computeOutput = [] (const process_input& pr, unsigned int& clock) {
process_ouput res;
res.m_responseTime = clock - pr.m_arrivalTime;
clock += pr.m_burstTime;
res.m_completionTime = clock;
res.m_turnaroundTime = res.m_completionTime - pr.m_arrivalTime;
res.m_waitingTime = res.m_turnaroundTime - pr.m_burstTime;
return res;
};
for (auto& input : inputs) {
res.push_back({input, computeOutput(input, clock)});
}
return res;
}
That changes main to:
int main()
{
std::vector<process> inputs {{0,0,5}, {1,1,3}, {2,2,8}, {3,3,6}};
auto procs = FCFS(inputs);
//Sort based on PID before showing result
std::sort(procs.begin(), procs.end(),
[](const process& p1, const process& p2) {
return p1.m_input.m_procId < p2.m_input.m_procId;
});
displayResult(procs);
}
Now which guaranties do we want to preserve?
process_input are simple data without internal constraints.process_output are simple data and seems not have internal constraints neither.process should have coherency between input and output: So no non-const-accesses to its members.// const member might be enough in that case,
// but I prefer to avoid const members
// which make class harder to use (not movable, not assignable, ....)
// so only const getters
struct process
{
public:
const process_input& input() const { return m_input;
const process_output& output() const { return m_outnput; }
private:
process_input m_input;
process_output m_output;
};
Alternatively or in addition to getter, you might add Display method here.