0
\$\begingroup\$

I am trying to set up a time variable as a local storage property to be retrieved when I open my app in my browser.

If I open it for the first time, today, I'll set it up to today at midnight.

Otherwise, if the time property has already been set up today in a previous session, I'll provide the retrieved value to the session, and assign Date.now to the local storage property for next time I'll visit my app.

I am trying to use ternary operators in place of if statements, but I see duplicated code I am not able to avoid. I am sure I am missing a trivial logic error, but I am not seeing it right now.

const NotificationsStorage: { lastCheck: number } = JSON.parse(localStorage.getItem('NotificationsStorage'));
const today: number = moment().startOf('day').valueOf();
const now: number = moment().valueOf();

localStorage
    ? NotificationsStorage
        ? (NotificationsStorage.lastCheck && NotificationsStorage.lastCheck > today)
            ? (
                this.lastCheck = NotificationsStorage.lastCheck,
                localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: now }))
            )
            : (
                localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: today })),
                this.lastCheck = today
            )
        : (
            localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: today })),
            this.lastCheck = today
        )
    : console.warn('Current browser doesn\'t support Local Storage API');
\$\endgroup\$
  • 1
    \$\begingroup\$ Is there a reason you are using ternary operators? If this will be production code, I would highly discourage nested ternaries of any sort as they're practically unmaintainable and the linter even warns you about it: eslint.org/docs/rules/no-nested-ternary \$\endgroup\$ – Srdjan Grubor May 9 '18 at 13:51
  • \$\begingroup\$ Hi @SrdjanGrubor , thanks for the comment, and for pointing out the issue of nested ternary operators. No specific reason at all, just trying to learn them, but I can easily see the issue of nested ternary operators. What would you suggest? Classic if/else statements? \$\endgroup\$ – caruso_g May 9 '18 at 13:55
  • \$\begingroup\$ Yes, classic if/else is almost always encouraged since it's easy to visually parse and maintain. As a sidenote, whatever speed it might seem that you're gaining with the ternary, the interpreter will most likely make that negligible of a difference but you lose almost all maintainability. \$\endgroup\$ – Srdjan Grubor May 9 '18 at 14:00
1
\$\begingroup\$

Whether you use ternaries or classic if/else you can often make your nesting easier to read by inverting your conditions. i.e.

! window.localStorage
    ? console.warn('Current browser doesn\'t support Local Storage API');             
    : NotificationsStorage
    && NotificationsStorage.lastCheck 
    && NotificationsStorage.lastCheck > today
        ? (
            this.lastCheck = NotificationsStorage.lastCheck,
            localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: now }))
        )
        : (
            localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: today })),
            this.lastCheck = today
        )

Although you can't do it with a ternary operator, I also usually prefer to use early returns to limit nesting. This also avoids the issue that you access localStorage in your first line even though it might not exist.

if (! window.localStorage) {
  console.warn('Current browser doesn\'t support Local Storage API');  
  return;
}

const NotificationsStorage: { lastCheck: number } = JSON.parse(localStorage.getItem('NotificationsStorage'));
const today: number = moment().startOf('day').valueOf();
const now: number = moment().valueOf(); 

if (NotificationsStorage
    && NotificationsStorage.lastCheck 
    && NotificationsStorage.lastCheck > today) {
      this.lastCheck = NotificationsStorage.lastCheck,
      localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: now }))
 } else { 
      localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: today })),
      this.lastCheck = today
}

You could simply that last if by separating the variables and the logic to update them:

const {newStoredValue, newLocalValue} = 
    NotificationsStorage
    && NotificationsStorage.lastCheck 
    && NotificationsStorage.lastCheck > today 
    ? [now, NotificationsStorage.lastCheck]
    : [today, today];

 localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: newStoredValue }));
 this.lastCheck = newLocalValue;
\$\endgroup\$
0
\$\begingroup\$

OK, I believe I found the solution. I was checking for something that was, instead, part of the bundle of conditions without which I would not been able to set up a lastCheck at all, so I put them together, hopefully, avoiding repetition.

Please, feel free to post a better solution. I would love to see it and learn.

const NotificationsStorage: { lastCheck: number } = JSON.parse(localStorage.getItem('NotificationsStorage'));
const today: number = moment().startOf('day').valueOf();
const now: number = moment().valueOf();

localStorage
    ? NotificationsStorage
    && NotificationsStorage.lastCheck 
    && NotificationsStorage.lastCheck > today
        ? (
            this.lastCheck = NotificationsStorage.lastCheck,
            localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: now }))
        )
        : (
            localStorage.setItem('NotificationsStorage', JSON.stringify({ lastCheck: today })),
            this.lastCheck = today
        )
    : console.warn('Current browser doesn\'t support Local Storage API');
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.