Multi-layer queue

Question

I must write code for my classes which simulates multi-layers production line. But I don't even know how to check if it's good. Can anyone take a look at my code, please? First machine works in random frequency putting integer into first queue. Then, second machine puts out this number at freq2, adding 100 and so on...

#include <stdio.h>
#include <iostream>
#include <queue>
#include <time.h>
#include <cstdlib>
using namespace std;
int main()
{
queue<int> k1;
queue<int> k2;
queue<int> k3;
int max1=5;
int max2=10;
int max3=2;
int freq1=5;
int freq2=3;
int freq3=4;
int loop=0;
float interwal=0.5;
while (1)
{
loop++;
int tmp=rand()%100;
int tmp2=rand()%5+1;
if (loop%tmp2==0 && k1.size()<max1) k1.push(tmp);
if (k1.empty()==false && k2.size()<max2 && loop%freq1==0)
{
 k2.push(k1.front()+100);
 k1.pop();
}
if (k2.empty()==false && k3.size()<max3 && loop%freq2==0)
{
 k3.push(k2.front()*100);
 k2.pop();
}
if (k3.empty()==false && loop%freq3==0)
{
 cout<<k3.front()<<endl;
 k3.pop();
}
clock_t ticks, ticks2;
ticks=clock();
ticks2=ticks;
while((ticks2/(float)CLOCKS_PER_SEC-ticks/(float)CLOCKS_PER_SEC)<interwal)
 ticks2=clock();
}
}

• Raystafarian: Does it work already? And you're looking to improve it? It may help to include what the exercise is as well as comment the code so it's easier to follow.
• SirPython: @Raystafarian The OP does not state that the code does not work; do you have reason to believe it is? The OP also states that they are looking to improve their code; do you believe otherwise?
• Raystafarian: @SirPython nope - "check if its good" was just the reason I asked if it worked already, as you know that's a requirement and it's best to clarify up front. I didn't intend to imply it doesn't work.

Answer 1

A lot here depends on how you define "good". On one hand, your code is fairly simple and straightforward. There are a few issues with readability:

1. indentation should be improved
2. if (x == false) should be written as if (!x)
3. Some of the names could be improved (e.g., k1, tmp, tmp2 are all pretty meaningless).

Those are pretty minor though.

Perhaps more importantly, it's not immediately clear how much of the code is intended to correspond to a particular part of the overall job. I'd tend to separate each sub-task into its own function, with its input and output queues as parameters to that function, so main would look more like:

while (1) { 
 generate(loop, k1);
 process1(loop, k1, k2);
 process2(loop, k2, k3);
 output(loop, k3);
 wait();
}

A still larger problem is that your code busy-waits to get the timing correct. This wastes a lot of CPU time (and on portable devices, a lot of battery power). At the very least, you probably want to look up std::sleep_for, which will make your waiting for the correct number of ticks quite a bit simpler.

The next step after that (though it's not clear to me that it's justified) would be for each process to execute as a separate thread, with thread-safe queues between the threads.