Other way to solve assigning

Question

Lets say I have collection of n workers. Lets say there are 3:

John
Adam
Mark

I want to know when they have to clean the office. If I set int cleanDays = 3 it would be something like that:

//Day of month;worker
1;John
2;John
3;John
4;Adam
5;Adam
6;Adam
7;Mark
8;Mark
9;Mark
10;John
11;John
.
.
.

If I set cleanDays = 1 it would be:

1;John
2;Adam
3;Mark
4;John
5;Adam
.
.
.

And so on.

I already managed something like this:

int cleanDays = 6;

for (int day=1; day<30;day++) {  //for each day
    int worker = (day-1 % cleanDays)%workers.Count; //get current worker (starting from index 0)
    for (int times=0; times< cleanDays; times++) //worker do the job `cleanDays` times
        Console.WriteLine(day++ + ";" +workers[worker].Name);
}

This is not working properly, because it gaves me 34 days. That because of day++ in first loop. But if I delete day++ from first loop:

for (int day=1; day<30;) {  //for each day
    int worker = (day-1 % cleanDays)%workers.Count; //get current worker (starting from index 0)
    for (int times=0; times< cleanDays; times++) //worker do the job `cleanDays` times
      Console.WriteLine(day++ + ";" +workers[worker].Name);
    }

It is giving output only with first worker. When I debugged I saw that:

int worker = (day-1 % cleanDays)%workers.Count;

and worker was equal to 0 everytime. That means: (20-1%6)%3 was equal to 0. Why does that happen?

Answer 1

UPDATE: I just read your question more carefully and realized you were not asking about the actual code at all. Your real question was:

That means: (20-1%6)%3 was equal to 0. Why does that happen?

First of all, it doesn't. (20-1%6)%3 is 1. But the logic is still wrong because you have the parentheses in the wrong place. You meant to write

int worker = (day - 1) % cleanDays % workers.Count;

Remember, multiplication, division and remainder operators are all higher precedence than subtraction. a + b * c is a + (b * c), not (a + b) * c. The same is true of - and %. a - b % c is a - (b % c), not (a - b) % c.

But I still stand by my original answer: you can eliminate the problem entirely by writing a query that represents your sequence operations, rather than a loop with a bunch of complicated arithmetic that is easy to get wrong.

Original answer follows.

---

Dmitry Bychenko's solution is pretty good but we can improve on it; modular arithmetic is not necessary here. Rather than indexing into the worker array, we can simply select-many from it directly:

var query = Enumerable.Repeat(
    workers.SelectMany(worker => Enumerable.Repeat(worker, cleanDays)),
    1000)
  .SelectMany(workerseq => workerseq)
  .Select((worker, index) => new { Worker = worker, Day = index + 1})
  .Take(30);

foreach(var x in query)
  Console.WriteLine($"Day {x.Day} Worker {x.Worker}");

Make sure you understand how this query works, because these are core operations of LINQ. We take a sequence of workers,

{A, B, C} 

This is projected onto a sequence of sequences:

{ {A, A}, {B, B}, {C, C} }

Which is flattened:

{A, A, B, B, C, C}

We then repeat that a thousand times:

{ { A, A, B, B, C, C },
  { A, A, B, B, C, C }, 
  ... 
} 

And then flatten that sequence-of-sequences:

{ A, A, B, B, C, C, A, A, B, B, C, C, ... }

We then select-with-index into that flattened sequence to produce a sequence of day, worker pairs.

{ {A, 1}, {A, 2}, {B, 3}, {B, 4}, ... }

Then take the first 30 of those. Then we execute the query and print the results.

Now, you might say isn't this inefficient? If we have, say, 4 workers, we put each on 5 days, and then we repeat that sequence 1000 times; that makes a sequence with 5 x 4 x 1000 = 20000 items, but we only need the first 30.

Do you see what is wrong with that logic?

LINQ sequences are constructed lazily. Because of the Take(30) we never construct more than 30 pairs in the first place. We could have repeated it a million times; doesn't matter. You say Take(30) and the sequence construction will stop constructing more items after you've printed 30 of them.

But don't stop there. Ask yourself how you can improve this code further.

The bit with the days as integers seems a bit dodgy. Surely what you want is actual dates.

var start = new DateTime(2017, 1, 1);

And now instead of selecting out numbers, we can select out dates:

  ...
  .Select((worker, index) => new { Worker = worker, Day = start.AddDays(index)})
  ...

What are the key takeaways here?

• Rather than messing around with loops and weird arithmetic, just construct queries that represent the shape of what you want. What do you want? Repeat each worker n times. Great, then there should be a line in your program somewhere that says Repeat(worker, n), and now your program looks like its specification. Now your program is more likely to be correct. And so on.

• Use the right data type for the job. Want to represent dates? Use DateTime, not int.

Answer 2

I would use a while loop, and use some tracking variables to keep track of which worker you are at and how many clean-times are left for that worker. Something like this:

const int cleanTime = 3; // or 1 or 6

var workers = new [] { "John", "Adam" , "Mark" }
var day = 1;
var currentWorker = 0;
var currentCleanTimeLeft = cleanTime;
while (day <= 30) {
    Console.WriteLine("{0};{1}", day, workers[currentWorker].Name);

    currentCleanTimeLeft--;
    if (currentCleanTimeLeft == 0) {
        currentCleanTimeLeft = cleanTime;
        currentWorker++;
        if (currentWorker >= workers.Length)
            currentWorker = 0;
    }   

    day++;
}

A very basic solution, no division or arithmatics required.