-2

I'm seeing this not wait during specified times.

I've been trying to learn c# and don't know exactly what all the syntax is yet. I just want to know if there are better ways of accomplishing this task.

while (DateTime.Now.DayOfWeek == DayOfWeek.Saturday 
    && DateTime.Now.DayOfWeek == DayOfWeek.Sunday 
    && DateTime.Now.Hour <=8 
    && DateTime.Now.Hour >=18  
    && DateTime.Now.Minute <=9)
{
    Random waitTime = new Random();
    int milliseconds = waitTime.Next(120000, 180000);
    System.Threading.Thread.Sleep(milliseconds);
}

I want it to wait while it is saturday, or sunday, if it before 0800, after 1800, or if it is in the first ten minutes of each hour.

The problem is, I've seen it exit the loop at 7:05pm on a monday, which is two conditions. Am I using the && function correctly, or should I replace it with something else?

  • 9
    You cannot be both Saturday AND Sunday...In addition `DateTime.Now.DayOfWeek == DayOfWeek.Saturday && DateTime.Now.DayOfWeek == DayOfWeek.Sunday` cannot be true in the planet we are in! Me thinks you want to read about the || (or operator) and understand boolean logic. C1 OR C2 means C1 or C2 can be true to evaluate to true. C1 AND C2 means C1 and C2 both must be true to evaluate to true. – JonH Jun 11 '19 at 17:03
  • 2
    Also, DateTime.Now.Hour can't be simultaneously less than 8 and greater than 18. – 15ee8f99-57ff-4f92-890c-b56153 Jun 11 '19 at 17:05
  • 3
    You'll want to learn about proper use of both `&&` and `||`, along with the order of precedence. Do you know what `&&` actually means/does? – Broots Waymb Jun 11 '19 at 17:06
  • 1
    Your logic uses all "and"s, but you have a number of "or"s in your description. – juharr Jun 11 '19 at 17:07
  • 6
    You *also* never need to create new Random generators in a loop, or your next question may be one that is rather frequently asked. – Ňɏssa Pøngjǣrdenlarp Jun 11 '19 at 17:08
  • @NatPongjardenlarp: Fortunately they finally changed the default implementation of `Random` so that creating one in a loop does not yield the same number over and over again. But it is still a bad practice. – Eric Lippert Jun 11 '19 at 17:34
  • 1
    The skill you need to learn to be successful at programming is *simplify the problem*. You say "I want it to wait while it is saturday or sunday..." ok, then write a program that does only one thing: *identifies whether it is Saturday or Sunday*. You cannot write the more complicated program until you can successfully write the simpler program, so *write the simpler program first*. You would have found your bug much faster had you tried to write that program. – Eric Lippert Jun 11 '19 at 17:35
  • Now, more fundamentally, this program is very broken; **this is not at all the correct way to wait until a condition is met.** – Eric Lippert Jun 11 '19 at 17:38
  • There are programs, and libraries, whose sole purpose is to schedule code that needs to run regularly under various, sometimes complicated, rules. You should really be using one of those systems rather than writing one from scratch, as you'll invariably find out that you haven't accounted for all of the things needed to do this task, as it's deceptively complicated to do properly. – Servy Jun 11 '19 at 17:41
  • @JonH Thank you, I'm pretty sure I just want to change all `&&` to `||` @EdPlunkett Yup, changing `&&` to `||` @BrootsWaymb First stab at implementing something I saw somewhere else. @juharr I realized that, and was asking for a solution. – ClaskoTheKnight Jun 11 '19 at 17:42
  • @NatPongjardenlarp Should I move `Random waitTime = new Random();` to outside the while loop? – ClaskoTheKnight Jun 11 '19 at 17:43
  • @EricLippert I had it broken into multiple sections, that worked properly, then I realized it was progressing through the individual parts, without checking the whole thing. Like if I had it check the minute before the day, it would wait at 0001, get to 0011, pass the minute check, wait till sunday is over, then run at 0001 on monday. So I want it checking if any of the conditions are true, so It has to be done in one line. – ClaskoTheKnight Jun 11 '19 at 17:48
  • @EricLippert Telling me not to do something and not suggesting an alternative is **utterly useless**. – ClaskoTheKnight Jun 11 '19 at 17:50
  • @Servy I've noticed it's complicated, that's why I'm asking peers for help. But that doesn't mean I can't get my exact intended behavior using another method. And it definitely doesn't mean it shouldn't be attempted. And, you didn't give me any specifics to go look at. – ClaskoTheKnight Jun 11 '19 at 17:53
  • @ClaskoTheKnight Rather than getting other people to fix one or two small bugs in your code, when there's an *enormous* amount of work left to do, you should simply *use an existing product that solves this problem*. Re-creating something like this from scratch is just not something you should be doing without good reason. You can do some research to find task scheduling tools for whatever system you're developing for, they're easy enough to find. Which one you want to use will depend on your programming environments and needed functionality. – Servy Jun 11 '19 at 18:05
  • @Servy That method would leave me without understanding what exactly it's doing, and why. I came here for knowledge, not for consumerism. – ClaskoTheKnight Jun 11 '19 at 18:20
  • @ClaskoTheKnight And yet you haven't gained that knowledge by asking this question either. All you got in the answers by asking this question is a bunch of code that's still not an effective implementation of what you want. If you want to actually learn how to accomplish this properly than *use an existing scheduling system*, because that's the proper way to solve this problem. If you want to understand how it works, most provide lots of documentation on what they're doing, and some will be open source if that's something you care about. – Servy Jun 11 '19 at 18:25
  • It's hard to know where to begin; the code is so wrong in so many ways; from the trivial, like confusing the meaning of "and" and "or", and then up to the large-scale structure of the program and it's intended problem space. If you had a friend who was trying to solve calculus problems but they showed through their work that they didn't understand the difference between addition and multiplication, how would you advise them to proceed? You need to step way, way back and get solid on the basics before you try to write a scheduler. – Eric Lippert Jun 11 '19 at 18:54
  • @Servy I have learned from this discussion. I learned that `||` means or. I learned that you should create variables outside of the loop they will be used in. I learned that you would rather throw an entire library at something rather than create the exact behavior manually. What if I only have 10240 bytes of free memory? I also learned that you can't block users on stack overflow. – ClaskoTheKnight Jun 11 '19 at 19:10
  • @EricLippert To make your analogy more apt, I had never learned `||`, that was the information I needed. How would I get that out of your comments? – ClaskoTheKnight Jun 11 '19 at 19:15
  • @ClaskoTheKnight If you wanted to know how to OR two booleans together why didn't you search for how to do that? It's an answer easily found, and not one best found by asking about how to schedule tasks. Saying you variables should be scoped outside of the loop is a very poor generalization. That you think using a library or product to solve a problem like this suggests *you are still not aware of all of the problems involved in this task*. As I've said, the problem is deceptively difficult to really get right. Do you really have a computer that only has 10k memory? I somehow doubt it. – Servy Jun 11 '19 at 19:21
  • @Servy I am new and did not know what I needed to know, the solution was an unknown unknown. I did not know what I needed to change. The simplest solution was to ask a question in regards to my exact problem, and needs. If you would like to help me, I posted more code below, tell me what would go wrong and why. – ClaskoTheKnight Jun 11 '19 at 19:41
  • @ClaskoTheKnight As far as boolean operators, your question asks what `&&` does, which is a question answered by simply looking up the documentation for that operator. And your question asks how to do something when a OR b OR C ... is true, so apparently you *did* know that you wanted to know to OR two conditions together, they weren't unknowns. Rather than trying to figure out how to re-write a lengthy and complicated process from scratch, and dealing with all of the problems in your code in production before realizing you need to fix them, you can instead use an existing working solution. – Servy Jun 11 '19 at 19:45
  • @ClaskoTheKnight: The important thing to get from comments is not the basic syntax of the language; you can learn that by reading literally any tutorial. The important thing to get out of these comments is *you are almost certainly attacking this task in a very wrong manner*. The fact that you don't know even the bare minimum necessary to write a C# program is not the problem per se; everyone was a beginner once, and you'll improve. The more fundamental problem is the unknown unknowns. Now you have a known unknown: *you don't know how to write a scheduler*; now you can research that. – Eric Lippert Jun 11 '19 at 20:33

3 Answers3

2

JonH is right it cannot be both Saturday and Sunday. Here's what your code should look like

            Random myRand = new Random();
        bool isSatOrSun = DateTime.Now.DayOfWeek == DayOfWeek.Saturday || DateTime.Now.DayOfWeek == DayOfWeek.Sunday;
        bool isNotBetween8And1800 = DateTime.Now.Hour <= 8 || DateTime.Now.Hour >= 18;
        bool isFirstTenMinutes = DateTime.Now.Minute <= 9;

        //if you want to do Between 8 & 18 on Saturday/Sundays OR first 10 minutes of each hour on Saturday/Sunday

        while(isSatOrSun && (isNotBetween8And1800 || isFirstTenMinutes))
        {
            //do your sleep
            int waitTime = myRand.Next(120000, 180000);
            System.Threading.Thread.Sleep(waitTime);

            //now recheck your variables
            isSatOrSun = DateTime.Now.DayOfWeek == DayOfWeek.Saturday || DateTime.Now.DayOfWeek == DayOfWeek.Sunday;
            isNotBetween8And1800 = DateTime.Now.Hour <= 8 || DateTime.Now.Hour >= 18;
            isFirstTenMinutes = DateTime.Now.Minute <= 9;
        }


        //if you want to do all day on Saturday/Sundays OR Between 8 & 18 any day of the week OR  first 10 minutes of each hour any day of the week

        while (isSatOrSun || isNotBetween8And1800 || isFirstTenMinutes)
        {
            //do your sleep
            int waitTime = myRand.Next(120000, 180000);
            System.Threading.Thread.Sleep(waitTime);

            //now recheck your variables
            isSatOrSun = DateTime.Now.DayOfWeek == DayOfWeek.Saturday || DateTime.Now.DayOfWeek == DayOfWeek.Sunday;
            isNotBetween8And1800 = DateTime.Now.Hour <= 8 || DateTime.Now.Hour >= 18;
            isFirstTenMinutes = DateTime.Now.Minute <= 9;
        }
chris-crush-code
  • 1,114
  • 2
  • 8
  • 17
  • 1
    Last OR may be incorrect depending on what he wants...He may want it to be only on saturdays and sundays...but not sure pretty vague. – JonH Jun 11 '19 at 17:09
  • @JonH yeah I'll update it with different options for different interpretations of what he's looking for. – chris-crush-code Jun 11 '19 at 17:12
  • @JonH I want it to pause if any of the conditions are true. – ClaskoTheKnight Jun 11 '19 at 17:56
  • @chris-crush-code Perfect!! This gives me so much information, I'll be able to modify this into exactly what I want. Teaching me about boolean variable code, and to double check. I love it. – ClaskoTheKnight Jun 11 '19 at 17:58
  • @chris-crush-code I'm a bit confused as how the last three lines are used to double check. Does it update `isSatOrSun` and the others to reflect the current time? Do you have to do that because you set those variables earlier? What if I put the expanded version instead of the boolean variable? – ClaskoTheKnight Jun 11 '19 at 18:05
  • @ClaskoTheKnight since the evaluation of the date & time is outside the while loop, you'll need to check if the conditions are still true at the end of the loop, otherwise you'll never exit out of the loop. The while loop is saying continue looping while these variables are true so if you never re-evaluate the variables, once you go into the loop you'll never get out since you've never set them to false. – chris-crush-code Jun 11 '19 at 18:16
  • @ClaskoTheKnight also, since you're new to programming, the biggest lesson to learn here is to understand EXACTLY what you're trying to do - if you can't spell it out in English (or whatever your native language may be) without any ambiguity, then you'll have a tough time translating your idea to code. – chris-crush-code Jun 11 '19 at 18:18
-1

Since you're learning I would advise that you use a boolean method for your logic and call it on the while function, like this:

Method:

public static bool IsWaitingTime()
{
 if ( ( DateTime.Now.DayOfWeek == DayOfWeek.Saturday  || DateTime.Now.DayOfWeek == DayOfWeek.Sunday )
 && ( DateTime.Now.Hour <= 8 || DateTime.Now.Hour >= 18 ) 
 && DateTime.Now.Minute <= 9 ) 
   return true;
else
   return false;
} 

While Loop:


while ( IsWaitingTime )
    {
        Random waitTime = new Random();
        int milliseconds = waitTime.Next(120000, 180000);
        System.Threading.Thread.Sleep(milliseconds);
    }

This is one approach, as @JonH said, a date can't be both Saturday and Sunday simultaneously (in the same timezone at least). chris-crush-code's anwser is also correct.

Ribeiro
  • 364
  • 1
  • 3
  • 12
-2

From what I've learned in the past 20 minutes, this seems like this'll pause if any of the following conditions are met; Is it sunday, is it saturday, Is it earlier than 0800, Is it later than 1800, Is the minute less than 10

Random waitTime = new Random();
while (DateTime.Now.DayOfWeek == DayOfWeek.Saturday || DateTime.Now.DayOfWeek == DayOfWeek.Sunday || DateTime.Now.Hour <=8 || DateTime.Now.Hour >=18  || DateTime.Now.Minute <=9)
{
int milliseconds = waitTime.Next(120000, 180000);
System.Threading.Thread.Sleep(milliseconds);
}
  • Is this intended to be an answer to the question, a new question, or a commentary, or something else entirely? **Please only post answers in the answer section**. It is not at all clear that this is intended to answer the question, particularly since there is already an accepted answer. – Eric Lippert Jun 11 '19 at 20:35
  • @EricLippert This is intended to be an answer. I accepted chris-chrus-code's answer because it is the most broad, and useful answer. I usually post what I will be using in my script as an answer, to show anyone interested, and for potential feedback. – ClaskoTheKnight Jun 12 '19 at 14:42