3
\$\begingroup\$

This game has a few basic functions:

  1. ask users trivia questions
  2. based on user input, add or substract points.
  3. save game data when user inputs save.
  4. allow old user to log in and play the same old game.

I only have two main questions:

  1. In this game,I tried not to write too many things in the main.js and let most things happen in the background. However, that results in a somewhat messy structure. The user class and quiz class eventually have way too many interactions. (sendData() function in User.js )For example, when old user logs in, the user class has to updates all the quiz attributes. I wonder if this is creating too much dependency? Should I do something like "dependency injection" in the main.js?

  2. Because in javascript I can't just combine 2 objects together. I did some work to copy the old object through JSON and then combine them. Is this too unnecessary? Should I just focus on changing the data structure when I first saved the data? (save() function in Quiz.js)

If anyone could just skim over the code and give some general suggestions,that would be amazing..!

Main.js

"use strict";

var Question = require("./lib/question.js");
var Quiz = require("./lib/quiz.js");
var read = require('read');
var User = require("./lib/user.js");


var question1 = new Question("Which metal is heavier, silver or gold?","gold",1);
var question2 = new Question(" How many legs do butterflies have?","six",2);
var question3 = new Question("Which is the country with the most people?","china",3);
var question4 = new Question(" Which state is the biggest in the US?","alaska",4);
var question5 = new Question("Which country has the largest area of land?","russia",5);

var questions_set = [question1,question2,question3,question4,question5];


var quiz1 = new Quiz(questions_set);

var user = new User("anonymous",0,quiz1);


user.startQuiz();

// var options = {
//     prompt: question1.question

User.js

"use strict";

var read = require('read');
var fs = require("fs");


class User {
    constructor(name,points,quiz){
        this.name = name;
        this.points = points;
        this.quiz = quiz;
        this.data;
    }

    startQuiz(){
        this.loadData();


        var options = {
            prompt: "new user?  type yes or no"
        }

        var boundDirect = this.direct.bind(this);
        read(options,boundDirect);
    }

    loadData(){
        var that = this;
        var haha = this;
        var boundSendData = this.sendData.bind(this);
        fs.readFile("lib/data.txt",function(error,saved_data){
            if (error){
                console.log("\n new game no old data");
            }else{
                try {
                    that.data = JSON.parse(saved_data);
                    // console.log("-----------data after json-----------")
                    // console.log(that.data)
                    boundSendData();
                    // console.log("---------------rechecking in read-----------")
                    // console.log(haha.data);
                }catch(e){
                    console.log("\n no old data")
                }

                // console.log("--------------receiving data--------------" + that.data)
            }
        })


    }

    sendData(){
        // console.log("---------------------data before sending---------------")
        // console.log(this.data)

        var old_user = this.name;

        this.quiz.saved_data = this.data;
        this.quiz.questions_set = this.data[old_user][0].questions_set;
        this.quiz.question_index = this.data[old_user][0].question_index;
        this.quiz.points = this.data[old_user][0].points;
        this.quiz.award_points = this.data[old_user][0].award_points;
        this.quiz.bonus_index = this.data[old_user][0].bonus_index;

        console.log("-------------sending data-----------------------")
        console.log(this.quiz)

        this.quiz.ask();


    }

    direct(err, answer) {
        if (answer.toUpperCase() === "YES"){
            this.register();
        }else if(answer.toUpperCase() === "NO"){
            this.login();
        }else{
            console.log(" \n --------------------------")
            console.log("valid input pls")
        }

    }

    register(){
        var options = {
            prompt: "create your username: "
        }
        var boundSaveUsername = this.saveUsername.bind(this);
        read(options,boundSaveUsername);

    }


    login(){
        var boundCheckUser = this.check_user.bind(this);

        var options = {
            prompt: "What's your username?"
        }

        read(options,boundCheckUser);

    }

    saveUsername(err,name){
        this.name = name;
        this.quiz.setPlayer(this.name);
        this.quiz.ask();
    }



    check_user(err,name){



        if (name in this.data ){
            console.log(" \n --------------------log in-----------")
            console.log("you have successfully logged in!!")
            this.name = name;
            this.loadData();

        }else{
            console.log(" \n --------------------log in-----------")
            console.log("records not found pls try again or register:")

            var options = {
                prompt: "register or try?  input register or try"
            }

            var boundReroute = this.reroute.bind(this)
            read(options,boundReroute);
        }
    }

    reroute(err,answer){
        if (answer.toUpperCase() === "REGISTER"){
            this.register();
        }else{
            this.login();
        }

    }


}


module.exports = User;

Quiz.js

"use strict";

var read = require('read');
var fs = require("fs");

class Quiz {

    constructor(questions_set){
        this.questions_set = questions_set;
        this.question_index = 0;
        this.points = 0;
        this.bonus_index = this.generate_bonus_index();
        this.award_points = 1;
        this.saved_data = 0;
    }

    generate_bonus_index(){
        var bonus_index = Math.floor(Math.random() * 4.9);
        return bonus_index;
    }



    adjust_points_value(){
        if (this.question_index === this.bonus_index){
            console.log("--------------jack pot!-------------")
            console.log("bonus question is worth than 2 points! or you can loose 2 points too.")
            this.award_points = 2;
        }else{
            this.award_points = 1;
        }
    }

    setPlayer(player_name){
        this.player_name = player_name;
    }

    ask(){

        console.log("----------------current game data--------------------")
        console.log(this.question_index)


        if (this.check()){
            console.log("------------congratz!--------------")
            console.log("you have answered all the qustions!")
            return;
        }

        // console.log("------------check bonus_index-------" + this.bonus_index)
        // console.log("--------------check question_index" + this.question_index)

        this.adjust_points_value();



        // console.log("--------------index is ----------");
        // console.log(this.questions_set);
        // console.log(this.question_index);


        var options = {
            prompt: (this.questions_set[this.question_index].question)
        }
        var boundEvaluate = this.evaluate.bind(this);
        read(options,boundEvaluate);
        console.log("--------------waiting for your answers-----------");
        // var i = 4;
        // while (i >= 0){
        //  var current_question = this.questions_set[i];
        //  // console.log("--------------index is ----------");
        //  // console.log(i);
        //  var options = {
        //      prompt: current_question.question
        //  }
        //  read(options,this.displayName);
        //  i -- ;
        // }

    }

    evaluate(err,answer){
        // console.log("------------------current questions set is---------------")
        // console.log(this);
        if (answer.toUpperCase() === this.questions_set[this.question_index].answer.toUpperCase()){
            // this.questions_set.splice(this.question_index,1);
            this.points += this.award_points;
            this.question_index ++ ;
            console.log("-------------------------yeah!-------------------------")
            console.log("you have got the right answer. moving on to next question")
            console.log("currently you have " + this.points + " points \n")
            this.ask();
        }else if (answer.toUpperCase() === "EXIT"){
            console.log("---------------oops-------------");
            console.log("you have decided to exit the game looooser")

        }else if (answer.toUpperCase()=== "SAVE"){
            console.log(" ---------------------what---------------")
            console.log("you have decided to save the game and come back later looooser");
            this.save();

        }else{
            this.points -= this.award_points;
            console.log("--------------nay--------------");
            console.log("pls try again");
            console.log("currently you have " + this.points + " points \n ")

            this.ask()

        }
    }

    check(){
        if (this.question_index === 5){
            return true;
        }
    }


    save(){

        // console.log("the current player name is " + this.player_name);

        //  console.log("the current player name is " + self.player_name);
        var new_data = {};
        console.log("-------------chechking old -data----------")
        console.log(this.saved_data);
        if (this.saved_data === 0){


            new_data[this.player_name] = [];
            console.log("----------the current status----------") 
            // console.log(this);

            new_data[this.player_name].push(this);

            console.log(new_data[this.player_name]);

        }else{
            // worst data manip ever


            var cloneOfOldData = JSON.parse(JSON.stringify(this.saved_data));

            for (var key in cloneOfOldData){
                if (cloneOfOldData.hasOwnProperty(key)) {
                    new_data[key] = cloneOfOldData[key];
                 }
            }

            delete this.saved_data;

            var key_for_this_round = this.player_name;
            new_data[key_for_this_round] = this;


            console.log("------------------checking new data--------------");
            console.log(new_data)
            new_data[this.player_name] = [];
            new_data[this.player_name].push(this);
        }

        // console.log(data);
        var toBeSaved = JSON.stringify(new_data,null,4)
        console.log(toBeSaved);

        fs.writeFile("lib/data.txt",toBeSaved,(err)=>{
            if (err) throw err;
        });

    }
    // displayName (err, name){
 //     console.log("Your name is: " + name)
    // }



}


module.exports = Quiz;

full code on GitHub

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Your user class has too many responsibilities. It should not be responsible for persistence, registration, or routing, likewise the quiz class should not be responsible for saving its results. In general, it is better not force classes into your design. Also, consider how the quiz and the user both save data. This is a serious problem. Load and save should be external to all business logic classes whenever possible. \$\endgroup\$ Commented Jul 4, 2016 at 23:26
  • \$\begingroup\$ @AluanHaddad What do you mean by "external to all business logic classes". Do you mean I should create an additional class just for saving the data? Also additional classes for all the other responsibilities you have mentioned(persistence,registration,routing)? I have heard of the single responsibility principle but I haven't practiced enough to really apply it in my little programs \$\endgroup\$
    – whales
    Commented Jul 5, 2016 at 13:26
  • \$\begingroup\$ Yes I do suggest that you move all persistence into logical units that are external to quizzes and users. These don't have to be classes, they could functions. Routing definitely should be handled separately, and perhaps interact with the persistence mechanisms such that data is loaded and saved as part of logical transitions between operations such as registration and quiz taking. \$\endgroup\$ Commented Jul 9, 2016 at 20:14

1 Answer 1

1
\$\begingroup\$

Unnecessary variables in Main.js

You have some unnecessary variables in your Main.js file. Because most things in JavaScript can be passed as parameters to functions/methods (I believe the proper term is "first-class citizen"), you don't need to store intermediate variables to pass info around.

Here's the simplified version of your Main.js:

"use strict";

var Question = require("./lib/question.js");
var Quiz = require("./lib/quiz.js");
var read = require('read');
var User = require("./lib/user.js");

var quiz = new Quiz([new Question("Which metal is heavier, silver or gold?","gold",1), 
                     new Question(...),
                     new Question(...)
                     ...
                    ]);

var user = new User("anonymous",0,quiz);
user.startQuiz();

You could even eliminate the quiz variable, but that'd be a little much.

From first glance, it looks like you've got some nice splitting up of modules here.


User starting with points

class User {
    constructor(name,points,quiz){
        this.name = name;
        this.points = points;
        this.quiz = quiz;
        this.data;
    }

In my opinion, it doesn't make sense why a user would start with points other than 0 in a quiz game. But, if that ends up making sense to you later, you should at least make that parameter optional:

class User {
    constructor(name, quiz, points = 0){
        this.name = name;
        this.quiz = quiz;
        this.points = points;
    }

Note: I removed the last line because you weren't actually setting this.data to anything.

Now, you can just do this:

var user = new User("anonymous", myQuiz);

And, if you feel that the user will always start with 0 points, just remove the points part from the argument list.


Variable names

var haha = this;

This is not an appropriate name, nor is this variable used anywhere. Looks like you meant to delete this.

While on the subject of deleting unused parts, you should go through and delete all of your commented out console.logs and excess/unnecessary blank lines.


This is all I have time for right now. Maybe I'll add more later.

\$\endgroup\$
1
  • \$\begingroup\$ thanks a lot! I will pay more attention to these details in the future! \$\endgroup\$
    – whales
    Commented Jul 6, 2016 at 3:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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