0

I want to create a script that will check if the time is between 10am and 10pm for weekdays and between 10am and midnight on weekends. After it does this I eventually want it so that it will display on a website "we are open till (depends if its weekday or not)" or "we open at 10am" if they are closed.The alert box is temporary while I try and figure this out. Heres what I have so far.

I have edited the mistakes pointed out by CrakC, I am now trying to add text to a div with the id 'areWeOpen'

also should this be in the head or body? It should be called before the id is read by the browser right?

   function checkIfOpen() {
     var now = new Date();
     var day = now.getDay();
     var time = now.getHours();
     var open, weekday, ;
     var closed = "We will open at 10Am";
     var openWeekday = "We are open till 10Pm";
     var openWeekend = "We are open till midnight";

     if (time >= 10) {
       var open = true;
     } else {
       var open = false;
     }

     if (day >= 1 && day <= 4) {
       var weekday = true;
     } else {
       var weekday = false;
     }
     if (open === true && weekday === true) {
       document.getElementById('areWeOpen').innerHTML(openWeekday);
     } else if (open === true && weekday === false) {
       document.getElementById('areWeOpen').innerHTML(openWeekend);
     } else {

       document.getElementById('areWeOpen').innerHTML(closed);
     }
   };
   checkIfOpen();
Jeff
  • 62
  • 11

3 Answers3

2

You can save many lines of code by simply setting the default values for when you're closed, then changing those values based on your checks. This is also more readable and maintainable.

innerHTML and textContent are properties, not methods. What this means is that you assign things to them instead of trying to call them using brackets as you tried. However, if you are just inserting text then you should use textContent.

(function checkIfOpen() {
    var now = new Date();
    var day = now.getDay();
    var hour = now.getHours();
  
    var otime = 10;
    var ctime = 24;
    var msg = "we open at "+otime+"am";
    var until = "midnight";
  
    if (day >= 1 && day <= 4) ctime = 22, until = "10pm";
    if (hour >= otime && hour <= ctime) msg = "we are open until " + until;
  
    document.getElementById('areWeOpen').textContent = msg;
})();
<div id="areWeOpen"></div>
  • I would except the open closing times change depending on what day of the week it is. – Jeff Oct 25 '15 at 02:20
  • Ok I see what you did and I do like that. A lot more simpler than what I had written. Thats one of my biggest problems with programming. I don't have a good grasp on writing clean code. My code always seems to be long and messy. Now i need to figure out how to write that "msg" into a specific div. I tried something above but it isn't working like I had thought it would. – Jeff Oct 25 '15 at 06:13
  • @TinyGiant when the value of `ctime` is 24, it shows `we are open until 12pm`. shouldn't that be `12am`? – Swastik Padhi Oct 25 '15 at 15:58
  • @CrakC Updated. I chose "midnight" instead because it is less ambiguous. –  Oct 25 '15 at 17:09
1

This should solve your problem. You had many mistakes in your code-

function checkIfOpen()
    {
        var now = new Date();
        var day = now.getDay();
        var time = now.getHours();
        var open, weekday; //declaring variables here so that there are accessible outside the two 'if's

        if (time >= 10) {
            open = true;
        } else {
            open = false;
        } //no semi-colon required here

        if (day >= 1 && day <= 4) {
            weekday = true;
        } else {
            weekday = false;
        } //no semi-colon required here

        if (open === true && weekday === true) //using === for comparision instead of ==
        {
            alert("we are open till 10pm");
        }
        else if (open === true && weekday === false) //using 'else if' instead of `if else`
        {
            alert("we are open till midnight"); //semi-colon required here
        }
        else
        {
            alert("we open at 10am");
        }
    }

JS Bin- http://jsbin.com/fujazovobo/edit?html,js,output

Swastik Padhi
  • 1,849
  • 15
  • 27
  • this didn't seem to change anything other than pointing out i accidentally put "if else" instead of "else if". I still don't get an alert of any kind – Jeff Oct 25 '15 at 02:08
  • you don't get an alert because you have defined only the function. next, you need to call it from within the html. let me update my answer with the `jsbin` link. – Swastik Padhi Oct 25 '15 at 02:10
  • Ah thanks I forgot all about calling the function. When i first made it I did it without making it all a function so I didn't need to call the function. So if I wanted to remove the alerts and instead add text to a div with a specific id and have it to update automatically when the page loads would I need all of this in a function or should just have a function that inserts the desired text based off the last 3 comparisons? – Jeff Oct 25 '15 at 02:18
  • @Anomaly well, they are the same both ways in your case as you want the function to execute and change the content of the `div` on every page load so either ways, the same code will be executed and I don't think you need the variables for some other purpose as in that case keeping only the conditions inside the function would be proper. – Swastik Padhi Oct 25 '15 at 02:25
  • I am trying to now get it to insert text into a div depending on the results. What am I doing wrong? – Jeff Oct 25 '15 at 03:17
0

It looks like you are well on your way to answering your own question. For time wrangling you should consider momentjs as it makes it much easier to work with time. For writing to the DOM jQuery is always an option. If you don‘t want to use a library try:

<div>Your message is <span id="message"></span></div>
<script>
  var textNode = document.createTextNode('text')
  document.getElementById('message').appendNode(textNode)
</script>

If at all possible, however, try not to do this in client side JavaScript. It will be more accessible to screen readers and search engines if you render this information on the server. If you are authoring static HTML files then the recommendations above should accomplish what you need.

Calebmer
  • 2,972
  • 6
  • 29
  • 36
  • so should I do something like this in php instead? – Jeff Oct 25 '15 at 02:07
  • I would highly recommend it. – Calebmer Oct 25 '15 at 02:07
  • I will definitely consider it. Now that I have started this I would like to finish it with Js just to get the experience doing it. If I were to do it in Php I only know how to get the day of the week written out (i.e. "Tuesday") I'm not sure I can pick up as quickly how to compare everything to determine the closing time. – Jeff Oct 25 '15 at 02:23
  • If it‘s valuable to your business for web crawlers (such as Google's SEO engines) to be able to access your site, you should definitely consider it. The one thing I will say is that [time](http://php.net/manual/en/function.date.php) in PHP is one of my favorite implementations :) – Calebmer Oct 25 '15 at 02:33
  • its actually not a big issue for crawlers to crawl this particular bit. it is displayed in the footer and other places. This was just going to be on the home page and something that showed a message if we are open or not. – Jeff Oct 25 '15 at 03:12