Is it ok to use a generator function to replace a promise management?

Question

For this code example, you have to imagine a series of animations on moving a robot (move left / right, go forward)

In reality it is a site with more complicated animations (loading ajax, loading images, multiple animations, etc ..) that I currently manage with promises, but as the site evolves, the code of this part becomes a dish of spagettti.

This is the first time I do something like this, and I wonder if this is really a good idea, because this way of doing things seems really strange for me.
I have the impression that I will eventually find myself with insoluble problems.
In any case my current site becomes a real nightmare, because I have to change some animations, add new ones ...

Does this sample code look correct?
Should I change something there?

const Root   = document.documentElement
  ,   gRoot  = getComputedStyle(Root)
  ,   moving = [ {T:-30,L:0},  {T:0,L:+30}, {T:+30,L:0}, {T:0,L:-30} ]
  ;
var RotateDeg = 0
  , RotateMov = 0
  , posT      = parseInt(gRoot.getPropertyValue('--PosT'))
  , posL      = parseInt(gRoot.getPropertyValue('--PosL'))
  ;
function F_1() // move forward
  {
  posT += moving[RotateMov].T
  posL += moving[RotateMov].L

  Root.style.setProperty('--PosT', posT + "px")
  Root.style.setProperty('--PosL', posL + "px")
  }
function T_L() // turn Left
  {
  RotateMov = (RotateMov +3) %4
  RotateDeg -=90
  Root.style.setProperty('--turn', RotateDeg + "deg")
  }
function T_R() // turn Right
  {
  RotateMov = (RotateMov +1) %4
  RotateDeg +=90
  Root.style.setProperty('--turn', RotateDeg + "deg")
  }
function R_0() // rotate to zero
  {
  RotateMov = 0
  RotateDeg = 0
  Root.style.setProperty('--turn', RotateDeg + "deg")
  }
function disableButtons(OnOff)
  {
  Bt_Tab_A.disabled = OnOff
  Bt_Tab_B.disabled = OnOff
  }
function* Sequence(Tab_fct)
  {
  for( let fct of Tab_fct) yield fct
  }

var iterator = Sequence([])

function nextSequence()
  {
  let command = iterator.next()
  if (!command.done) command.value()
  else disableButtons(false)
  }

Bt_Tab_A.onclick=_=>
  { 
  disableButtons(true)
  iterator = Sequence( [ F_1, T_L, F_1, T_R, F_1, T_R, F_1, F_1, T_R, F_1, F_1, T_R, F_1, R_0  ] )
  nextSequence()
  }
Bt_Tab_B.onclick=_=>
  { 
  disableButtons(true)
  iterator = Sequence( [ T_L, F_1, T_R, F_1, T_R, F_1, T_R, F_1, R_0 ] )
  nextSequence()
  }
robot.addEventListener('transitionend',  nextSequence )

:root {
  --turn  : 0deg;
  --PosT  : 110px;
  --PosL  : 90px;
}
#robot {
  font-size   : 16px;
  width       : 30px;
  height      : 30px;
  background-color: aqua;
  text-align  : center;
  line-height : 1.8em;
  transition  : all .5s linear;
  transform   : rotate( var(--turn) );
  position:fixed;
  top : var(--PosT);
  left: var(--PosL);
}

<div id="robot">R</div>

<button id="Bt_Tab_A"> Sequence A</button>
<button id="Bt_Tab_B"> Sequence B</button>

Tips and advice are welcome;)

No, it's not fine. Unless you need to directly control generators, you will want to use `async`/`await` instead. And you still should use promises for the indivudual async steps, instead of calling back some master `nextSequence` function. — Bergi, Jun 30 '19 at 14:57
No, that `nextSequence` is a nightmare because it's a global state that you rely on everywhere. Promises are much cleaner. You can just put the animation functions that return promises in an array, [and run them sequentially](https://stackoverflow.com/a/30823708/1048572). Or just write a `function sequenceA(){ await Forward(); await Forward(); await TurnLeft(); await Forward(); … }`. Much simpler than anything with generators. — Bergi, Jun 30 '19 at 18:13
For simplicity, you might consider (1) each sequence instruction to specify a complete vector (direction and distance) (2) turns to be made automatically rather than having to code them yourself. For example, your Bt_Tab_A sequence might be `Sequence(['U_1', 'L_1', 'U_1', 'R_2', 'D_2', 'L_1', 'U_0'])`. It would be fairly trivial to parse out each instruction into its *direction* and *distance* components then act on them. This isn't really an answer to the question as it stands but if you were to ask another question ... — Roamer-1888, Jul 02 '19 at 22:34
The principle of what I described above might still be useful (to someone), namely to form sequences of multi-parameter instructions. My suggestion uses strings but you could equally use an array of objects or functions with bound-in params. The approach would be generally applicable, not just to a robot graphic. — Roamer-1888, Jul 03 '19 at 01:20
@Roamer-1888 your idea assumes that we can establish a list of actions that are common to all objects, except this is absolutely not the case — Mister Jojo, Jul 03 '19 at 01:28
I don't think I have made that assumption but there again I know very little about your project. — Roamer-1888, Jul 03 '19 at 01:48

yqlim · Accepted Answer · 2019-07-01T07:57:32.730

In your case, I feel like Promise is the way to go.

In my own rule of thumb (beware, it's opinionated):

Use promise when you need to execute async operations sequentially.
Use generator function when you need to generate result when you need it.

In your snippet, all you need to do is to have the animations run sequentially, not when you need it.

Besides, you will notice you have multiple functions that are dependant on the current state (result) of a single variable (i.e., iterator). What's bad about this is that when there's a bug in between your sequence, you will take more time and effort to debug the situation because one change in one function might affect the other functions. You also have a global transitionend event listener that is quite consufing at the first glance.

In short, using generator functions, the flow of sequential operations is hard to understand.

The approach below is using async/await. Only Sequence and nextSequence methods are modified (comment explanations inside). Every operation is contained within its own function scope. Reliant of global variables is reduced:

(Sorry I formatted the code to my code style when I wrote them)

const Root = document.documentElement;
const gRoot = window.getComputedStyle(Root);
const moving = [
  {
    T: -30,
    L: 0
  },
  {
    T: 0,
    L: +30
  },
  {
    T: +30,
    L: 0
  },
  {
    T: 0,
    L: -30
  }
];

let RotateDeg = 0;
let RotateMov = 0;
let posT = parseInt(gRoot.getPropertyValue('--PosT'));
let posL = parseInt(gRoot.getPropertyValue('--PosL'));

function F_1(){
  posT += moving[RotateMov].T;
  posL += moving[RotateMov].L;

  Root.style.setProperty('--PosT', posT + 'px');
  Root.style.setProperty('--PosL', posL + 'px');
}

function T_L(){
  RotateMov = (RotateMov + 3) % 4;
  RotateDeg -= 90;
  Root.style.setProperty('--turn', RotateDeg + 'deg');
}

function T_R(){
  RotateMov = (RotateMov + 1) % 4;
  RotateDeg += 90;
  Root.style.setProperty('--turn', RotateDeg + 'deg');
}

function R_0(){
  RotateMov = 0;
  RotateDeg = 0;
  Root.style.setProperty('--turn', RotateDeg + 'deg');
}

function disableButtons(OnOff){
  Bt_Tab_A.disabled = OnOff
  Bt_Tab_B.disabled = OnOff
}

async function Sequence(Tab_fct){
  // Disable buttons before start
  disableButtons(true);
  
  for (let fct of Tab_fct)
    // Run the animation one by one
    await nextSequence(fct);

  // Reenable buttons before end
  disableButtons(false);
}

function nextSequence(fct){
  return new Promise(res => {
    // Move event listener here so that they dont depend on a global one.
    // Use { once: true } to run this callback only once
    window.addEventListener('transitionend', res, { once: true });
    
    // Run the animation
    fct();
  })
}


Bt_Tab_A.onclick = () => {
  Sequence([F_1, T_L, F_1, T_R, F_1, T_R, F_1, F_1, T_R, F_1, F_1, T_R, F_1, R_0]);
}

Bt_Tab_B.onclick = () => {
  Sequence([ T_L, F_1, T_R, F_1, T_R, F_1, T_R, F_1, R_0 ]);
}

:root {
  --turn  : 0deg;
  --PosT  : 110px;
  --PosL  : 90px;
}
#robot {
  font-size   : 16px;
  width       : 30px;
  height      : 30px;
  background-color: aqua;
  text-align  : center;
  line-height : 1.8em;
  transition  : all .5s linear;
  transform   : rotate( var(--turn) );
  position:fixed;
  top : var(--PosT);
  left: var(--PosL);
}

<div id="robot">R</div>

<button id="Bt_Tab_A">Sequence A</button>
<button id="Bt_Tab_B">Sequence B</button>

@Bergi Actually, you're right. I'll update the answer, thanks. — yqlim, Jul 01 '19 at 07:53
Thank you, your code is very clear, and even shorter than mine! (despite the awful style;)) I think I will have to adapt it to my mix of different elements to animate, with their different resoles (`transitionend`, `ajax.success` ...) ps:`once:true` is a really plus for me! — Mister Jojo, Jul 01 '19 at 11:16
Glad it helped @MisterJojo (even though I had to read through _your_ style ;)). Though you might want to check the browser support for `once: true` — yqlim, Jul 01 '19 at 11:40
it's not really mine : https://en.wikipedia.org/wiki/Indentation_style#Whitesmiths_style — Mister Jojo, Jul 01 '19 at 12:29

Is it ok to use a generator function to replace a promise management?

1 Answers1

Linked