Veljko Stefanovic
Veljko Stefanovic

Reputation: 511

FCC Pomodoro Clock failing in-built tests

I'm doing FCC exercise Pomodoro Clock that sort of works. I think it’s the problem that when countdown was in it’s last minute, pause won’t work. Btw pause works if it’s not last minute. My suspicion is that this problem causes other tests to fail. Also test say that reset functionality doesn’t work, but it works.

Edit: Deeper explanation of my problem: Method paused() is used to change state paused from false to true and vice-versa. That method is called inside a timer() method whose job also is to initiate setInterval. setInterval is been set in variable this.clearTimer. If it’s paused true then clearInterval() is initiated and timer is stopped.

When element with start_stop id is clicked, paused changes, evaluation occurs and as i said it before; false => setInterval goes; true => setInterval stops.

When unpaused, setInterval from values recorded in state. Problem is that when you pause timer which below one minute(59, 58 etc. sec.), it won’t resume countdown despite state of paused being changed from true to false?! If it’s countdown above 1min., start/pause works as prescribed.

You can see the code at: my pen

Here is the code of main component(from local source):

    import React from 'react';
    import ReactDOM from 'react-dom';
    import 'bootstrap/dist/css/bootstrap.css';
    import './style.scss';
    import Item from "./item.js";
    import Arrow from "./arrow.js";
    import Fa from "./fa.js";

    class Pomodoro  extends React.Component {

    constructor(props) {
        super(props);
        this.state = {
            breakLength: 5,
            multiplier: 25,
            base: 1000,
            time: 0,
            minutes: 25,
            seconds: 60,
            paused: false,
            session: false,
            break: true,
            disabled: false,
        };
        this.increment = this.increment.bind(this);
        this.decrement = this.decrement.bind(this);
        this.timer = this.timer.bind(this);
        this.reset = this.reset.bind(this);
        this.paused = this.paused.bind(this);
        this.myRef = React.createRef();
        this.clearTimer = null;
    }

    componentDidMount(){

        this.setState({
            time: ""+(this.state.multiplier*this.state.base*60),
            minutes: this.state.multiplier,
            seconds: this.state.seconds === 60 ? "00" : this.state.seconds ,
        });

    }

    paused(){

        this.setState({
            paused: !this.state.paused
        });

    }

    timer(){

        this.paused();

        if(this.state.paused === false){

            if((this.state.minutes!==0 && this.state.seconds!==0) || (this.state.seconds!==0)){

                this.clearTimer = setInterval(() => {

                    if(this.state.session===false){
                        console.log("Sada ide session.");

                        this.setState({
                            time: ""+(this.state.time-this.state.base),
                            minutes: this.state.minutes > 0 ? Math.floor(((this.state.time-this.state.base)/(this.state.base))/60) : this.state.minutes,
                            seconds: this.state.seconds > 0 ? this.state.seconds-1 : 59,
                            session: (this.state.minutes===0 && this.state.seconds===1) ? true : false,
                            break: (this.state.minutes===0 && this.state.seconds===1) ? false : true,
                        });

                    }

                    if(this.state.break===false && this.state.session===true && this.state.time==="0"){
                        console.log("Kraj session-a. Sada ide resetovanje, pa break.");
                        this.setState({
                            time: ""+(this.state.breakLength*this.state.base*60),
                            minutes: this.state.breakLength,
                            seconds: 60,
                        });

                    }

                    if(this.state.break===false){
                        console.log("Sada ide break.");
                        this.setState({
                            time: ""+(this.state.time-this.state.base),
                            minutes: this.state.minutes > 0 ? Math.floor(((this.state.time-this.state.base)/(this.state.base))/60) : this.state.minutes,
                            seconds: this.state.seconds > 0 ? this.state.seconds-1 : 59,
                            session: (this.state.minutes===0 && this.state.seconds===1) ? false : true,
                            break: (this.state.minutes===0 && this.state.seconds===1) ? true : false,
                        });

                    }

                    if(this.state.break===true && this.state.session===false && this.state.time==="0"){
                        console.log("Kraj break-a. Sada ide resetovanje, pa session.");
                        this.setState({
                            time: ""+(this.state.multiplier*this.state.base*60),
                            minutes: this.state.multiplier,
                            seconds: this.state.seconds === 60 ? "00" : this.state.seconds,
                        });

                    }

                }, this.state.base);

            }

        }
        else{

            clearInterval(this.clearTimer);

        }

    }

    reset(){

        this.myRef.current.pause();
        this.myRef.current.currentTime = 0;
        clearInterval(this.clearTimer);
        this.clearTimer = null;
        this.setState({
            breakLength: 5,
            multiplier: 25,
            base: 1000,
            time: ""+(25*1000*60),
            minutes: 25,
            seconds: 60,
            paused: false,
            session: false,
            break: true,
            disabled: false,
        });

    }

    increment(e){

        console.log(e.target.id);
        let myId = e.target.id;

        if(myId==="break-increment"){

            this.setState({
                breakLength: this.state.breakLength <60 ? this.state.breakLength+1 : this.state.breakLength,
            });

        }
        else if(myId==="session-increment"){

            this.setState({
                multiplier: this.state.multiplier < 60 ? this.state.multiplier+1 : this.state.multiplier,
                time: this.state.time !== "60" ? ""+((this.state.multiplier+1)*this.state.base*60) : this.state.time,
                minutes: this.state.minutes < 60 ? this.state.multiplier+1 : this.state.minutes,
            });

        }

    }

    decrement(e){

        console.log(e.target.id);
        let myId = e.target.id;

        if(myId==="break-decrement" && this.state.breakLength > 1){

            this.setState({
                breakLength: this.state.breakLength > 1 ? this.state.breakLength-1 : this.state.breakLength,
            });

        }
        else if(myId==="session-decrement" && this.state.multiplier > 1 && this.state.time > 1 && this.state.minutes > 1){

            this.setState({
                multiplier: this.state.multiplier > 1 ? this.state.multiplier-1 : this.state.multiplier,
                time: this.state.time > 1 ? (""+((this.state.multiplier-1)*this.state.base*60)) : this.state.time,
                minutes: this.state.minutes > 1 ? this.state.multiplier-1: this.state.minutes,
            });

        }

    }

    render(){
        //console.log(this.state);

        const minutes = (""+this.state.minutes).length===1 ? "0"+this.state.minutes : this.state.minutes;
        const seconds = this.state.seconds===60 ? "00" : ((""+this.state.seconds).length===1 ? "0"+this.state.seconds : this.state.seconds);
        const time = minutes+":"+seconds;

        if(time==="00:00"){
            console.log("1: "+time);
            console.log("2: "+this.state.minutes+":"+this.state.seconds);
            this.myRef.current.play();
        }

        /*if((this.state.minutes+":"+this.state.seconds)===time){
            alert("alert2: "+this.state.minutes+":"+this.state.seconds);
        }*/

        const lastSesMin = (minutes==="00") ? {color: 'red',} : {};

        const decrement = this.clearTimer ? ()=>{} : this.decrement;
        const increment = this.clearTimer ? ()=>{} : this.increment;

        const item2Head = <h3 id="break-label">Break Length</h3>;
        const fa1 = <Fa klasa={"fa fa-arrow-down fa-2x"} id={"break-decrement"} onClick={decrement}/>;
        const fa2 = <Fa klasa={"fa fa-arrow-up fa-2x"} id={"break-increment"} onClick={increment}/>;
        const arr1 = [<Arrow klasa={"arrow"} key={0} arrow={item2Head}/>, <br key={1}/>, <Arrow klasa={"arrow"} key={2} arrow={fa1}/>, <Arrow id={"break-length"} klasa={"nums"} key={3} arrow={this.state.breakLength}/> , <Arrow key={4} klasa={"arrow"} arrow={fa2}/>];

        const item3Head = <h3 id="session-label">Session Length</h3>;
        const fa3 = <Fa klasa={"fa fa-arrow-down fa-2x"} id={"session-decrement"} onClick={decrement}/>;
        const fa4 = <Fa klasa={"fa fa-arrow-up fa-2x"} id={"session-increment"} onClick={increment}/>;
        const arr2 = [<Arrow klasa={"arrow"} key={0} arrow={item3Head}/>, <br key={1}/>, <Arrow klasa={"arrow"} key={2} arrow={fa3}/>, <Arrow klasa={"nums"} id={"session-length"} key={3} arrow={this.state.multiplier}/> , <Arrow key={4} klasa={"arrow"} arrow={fa4}/>];

        const countdownLabel = (this.state.session===false &&  this.state.break===true) ? "Session" : "Break";
        const item4Head = <h3 key={0} id={"timer-label"} style={lastSesMin}>{countdownLabel}</h3>;
        const nums2 = <div key={1} className="nums" style={lastSesMin} id={"time-left"}>{time}</div>;
        const arr3 = [item4Head, nums2];

        const fa5 = <Fa key={0} klasa={"fa fa-play arrow controls"} title={"start-pause"}/>;
        const fa6 = <Fa key={1} klasa={"fa fa-pause arrow controls"} title={"start-pause"}/>;
        const fa7 = <Fa key={2} klasa={"fa fa-refresh arrow controls"} id="reset" title={"reset"} onClick={this.reset}/>;
        const startPause = <div id="start_stop" key={4} onClick={this.timer}>{fa5}{fa6}</div>;
        const arr4 = [startPause, fa7];

        return(
            <div className="grid-container cent">

                <Item klasa={"item1"} arrowsAndNums={"Pomodoro Clock"}/>

                <Item klasa={"item2"} arrowsAndNums={arr1}/>

                <Item klasa={"item3"} arrowsAndNums={arr2}/>

                <Item klasa={"item4"} arrowsAndNums={arr3}/>

                <Item klasa={"item4"} arrowsAndNums={arr4}/>
                <audio ref={this.myRef} id="beep" src="http://soundbible.com/grab.php?id=2158&type=wav"></audio>
            </div>
        );

    }

}

    ReactDOM.render(<Pomodoro/>, document.getElementById('root'));

Edit2: I resolved pausing issue. I just changed line if(this.state.minutes!==0 && this.state.seconds!==0){ to line if((this.state.minutes!==0 && this.state.seconds!==0) || (this.state.seconds!==0)){.

Here are some images that show test errors even if there should be no errors. PS: For error testing of the exercise it has been used fcc's error testing script, that is what generates these supposed errors.

Image1 of the errors: Image1 of the errors

Image2 of the errors: enter image description here

Edit3: And image of an error which might come out of testing suite: enter image description here

Edit4: As i recently discovered, even FCC's pomodoro fails tests sometimes. Sometimes in different browsers, sometimes on sundays, sometimes etc. .... Long story short, i devised new code, that addresses issues in previous edits i made. Still, it suffers same problems as forementioned fcc's pomodoro. I read somewhere, something about timing events, whose implementation and execution depends on browsers and in this case test suite itself uses to, as also is my pomodoro. So there should be "conflicts" of sorts ... Resolving these inconsistencies and conflicts, so that in every browser, in every suit/test, looks insurmountable and i guess would ... What i'm trying to ask, is it ok if i submit my pomodoro, concerning everything being said? My improved pomodoro Added comments that explain functioning in the code so feel free to browse. Image of my test fail: failed test On the upper left you can see 28/29 passed tests. Notice: you can see updated code in codepen together with comments.

Upvotes: 3

Views: 773

Answers (1)

Sebastian B.
Sebastian B.

Reputation: 2191

Just read some code (without testing your sandbox), but I found a problem that can lead to some obscure bugs due to possible race conditions:

paused(){

    this.setState({
        paused: !this.state.paused
    });

}

timer(){

    this.paused();

    if(this.state.paused === false){

    // ...

timer() is calling paused, where the state is updated. Then timer() checks the new state.

Problems:

  1. setState may be batched, so consecutive calls to paused may be based on the some value.

Solution: use a function as state updater:

setState(prevState => ({
    paused: !prevState.paused
}));
  1. setState may be asynchronous (same link), so paused state may not reflect that change when being read in timer()!

Solution: use a callback as second parameter to setState():

setState(prevState => ({
    paused: !prevState.paused
}), () => {
    // You can read updated this.state.paused here....
});

So the whole code fragment could be implemented like this:

 paused(callback){
    setState(prevState => ({
        paused: !prevState.paused
    }), callback);
}

timer(){

    this.paused(() => {

        // if(this.state.paused === false){
        // ...
    });

But if you have many such places, you can quickly get in some kind of callback hell. And managing stopping/starting a timer in parallel to these state changes can become quite tricky by its own. You could at least simplify your timer but letting it always run (start in componentDidMount(), stop in componentWillUnmount()) and deciding on each tick what to do (e.g. doing nothing when being "paused", or letting a "pause" indicator blink ...)

Side note: variables like base do not need to be in state because they are just internal variables which don't need to trigger re-rendering of the component.

Upvotes: 2

Related Questions