My method for timers. Why is this not acceptable?

Hello everyone, I'm stuying robotics and my teacher allways says that using delay function is wrong because it stops the whole program for as long as the delay is functioning. He developed his own library for timers (and it works perfectly) for us to use, but it is confusing to me when I use it, so I came up with this idea:

The program in question is an Arduino-PLC communication bit per bit, which is binarily reading the value "167" in Arduino and sends it to Siemens S7-1200 PLC via output.

Please just focus on my timer anyways.

int value = 167;          //Value to transfer in BIN digits
int timer = 0;            //Timer variable
int mark_timer = 0;       //Marks second period of the timer
int read_bit = 0;         //Counter for which bit to read
int output_clock = 12;    //Output for when to send the data to S7-1200
int output_data = 13;     //Output data for S7-1200

void setup() {
Serial.begin(9600);
pinMode(output_clock,OUTPUT);
pinMode(output_data,OUTPUT);
Serial.println(value,BIN); //Shows the BIN digit to help at developing the code
}

void loop() {
if(timer <= 500){         //Whenever the timer is below 500ms, it will do nothing
   digitalWrite(output_clock,0);
   timer = timer + 1;     //adds 1ms each time we pass through the delay
   delay(1);
   }
if(timer >= 500){         //When the timer hits 500ms, it activates the data output
   Serial.print(bitRead(value,read_bit)); //Shows the bit that's going to be transferred to S7-1200
   digitalWrite(output_clock,1); //Enables the signal for S7-1200 to read (the other half of the reading goes at S7-1200's code)
   digitalWrite(output_data,bitRead(value,read_bit)); //Sends the bit to S7-1200
   read_bit = read_bit + 1; //Prepares the counter for reading the next bit when the timer re-activates
   mark_timer = 1;        //First half of the timer is completed
   }
if ((mark_timer == 1) && (timer <= 1000)){ //Waits for another half a second, this process is made in order to simulate the Manchester II method of reading data
   delay(1);
   }
if ((mark_timer = 1) && (timer >= 1000)){  //Resets the code for the next bit to be readed
   timer = 0;
   mark_timer = 0;
   }
if(read_bit == 8){        //Resets the counter to begin from first bit again when it has hit the 8th digit
   Serial.println();
   read_bit = 0;
   }
}

With this, I want to delay(1) as many times as it takes to get to 1 second, 1 minute...

As I see it, it won't stop the program since it's only running 1 millisecond every time it reads the sentence.

Do you think I am wrong? Maybe I should discuss this with my teacher? What do you think?

Thank you very much.

Why are you inventing a way to figure out how many millisecond? Use the millis function. That’s what it already does.

Yes, even 1ms of delay is almost 2000 instructions you blocked. You’ve got the right idea but there’s no reason to delay even for that one millisecond.

Delta_G:
Why are you inventing a way to figure out how many millisecond? Use the millis function. That’s what it already does.

Yes, even 1ms of delay is almost 2000 instructions you blocked. You’ve got the right idea but there’s no reason to delay even for that one millisecond.

Okay, I got it. Now that you mention it, is does make my code a lot more unefficient when I'm adding new functions. This method is hopelessly unefficient.

Thank you!!

The other problem is that you have multiple different branching paths that depend on a complex combination of internal values, and the delay is in two of those paths behind conditionals.

Whenever you have complex looping and branching pathways like this, you need to go over it with a fine-toothed comb to try to imagine every possible sequence of actions you can take through it and see what happens. And I found something happening that is probably not what you intended. What happens when timer is between 500 and 1000? Which path will your code follow on those 500 loops, and what will it do?

Jiggy-Ninja:
The other problem is that you have multiple different branching paths that depend on a complex combination of internal values, and the delay is in two of those paths behind conditionals.

Whenever you have complex looping and branching pathways like this, you need to go over it with a fine-toothed comb to try to imagine every possible sequence of actions you can take through it and see what happens. And I found something happening that is probably not what you intended. What happens when timer is between 500 and 1000? Which path will your code follow on those 500 loops, and what will it do?

I made the typical mistake of repeating the values in my conditionals. And what's more, I must put my conditional mark_timer at 0 at the 500ms conditions in order to avoid branching.

((mark_timer == 0) && (timer <= 500))
((mark_timer == 1) && (timer <= 1000))

Anyways, I'll just use my teacher's timer method from now.

Thank you very much!