Rock, paper, scissors game in expressjs typescript

Question

This is a simple 2 player game made in NodeJS and the ExpressJS framework with typescript. I've built using OOP approach to as far of an extent as I know of it. I'm really curious as to how further more I could improve it to satisfy even more of OOP principles and enhance extensibility.

import { IRuleSet, ruleSet } from './../shared/constants/ruleSet';
import { Player } from './player.model';

export type IGameResult = {
  player1Score?: number;
  player2Score?: number;
  message: string;
};

export class Game {
  ruleSet: IRuleSet;
  player1: Player;
  player2: Player;
  constructor(player1: Player, player2: Player) {
    this.player1 = player1;
    this.player2 = player2;
    this.ruleSet = ruleSet;
  }

    whoWins(player1Move: string, player2Move: string) {
      let result: IGameResult = {
        player1Score: undefined,
        player2Score: undefined,
        message: 'Invalid Choice',
      };
    if (ruleSet[player1Move.toLocaleLowerCase()].includes(player2Move)) {
      result = {
        player1Score: this.player1.incrementPoint(),
        player2Score: this.player2.getScore(),
        message: `Player 1 Wins with ${player1Move}`,
      };
      return result;
    } else if (ruleSet[player2Move.toLocaleLowerCase()].includes(player1Move)) {
      result = {
        player1Score: this.player1.getScore(),
        player2Score: this.player2.incrementPoint(),
        message: `Player 2 Wins with ${player2Move}`,
      };
      return result;
    } else {
      result = {
        player1Score: this.player1.getScore(),
        player2Score: this.player2.getScore(),
        message: 'Draw',
      };
      // Return a default value if no match is found

      return result;
    }
  }
}

// rules
export interface IRuleSet{
  [key: string]: string[];
}

export const ruleSet: IRuleSet = {
  rock: ['scissors'],
  scissors: ['paper'],
  paper: ['rock'],
};

// app.ts
import express, { Express } from 'express';
import morgan from 'morgan';
import setRoutes from './routes';
import dotenv from 'dotenv';
import bodyParser from 'body-parser';
const app: Express = express();

class Server {
  constructor(app: Express) {
    this.init();
    this.useMiddleware(app);
    this.setRoutes(app);
    this.listen(app);
  }

  init() {
    dotenv.config();
    app.set('view engine', 'ejs');
    app.set('views', __dirname + '/views');

    console.log('Starting Express Server...');
  }
  useMiddleware(app: Express) {
    app.use(express.json());
    app.use(bodyParser.urlencoded({ extended: true }));
    app.use(morgan('dev'));
    app.use(express.static(__dirname + '/public/styles'));
  }
  setRoutes(app: Express) {
    console.log('Setting up routes...');
    setRoutes(app);
  }
  listen(app: Express) {
    app.listen(process.env.PORT, () => {
      console.log(`Server listening on port ${process.env.PORT}`);
    });
  }
}

new Server(app);

// game.controller.ts

export class GameController {
  public async home(_: Request, res: Response) {
    res.render('home');
  }

  public play(req: Request, res: Response) {
    const { player1Move, player2Move, player1, player2 } = req.body;

    const game = new Game(player1, player2);

    const { player1Score, player2Score, message } = game.whoWins(
      player1Move,
      player2Move
    );

    res.render('game', {
      message,
      player1Score: player1Score || game.player1.getScore(),
      player2Score: player2Score || game.player2.getScore(),
    });
  }

  public newPlay(req: Request, res: Response) {
    const { player1, player2 } = req.body;
    const game = new Game(player1, player2);

    res.render('game', {
      message: '',
      player1Score: game.player1.resetScore(),
      player2Score: game.player2.resetScore(),
    });
  }
}

// middleware 

import { Player } from '../models/player.model';
import { Request, Response, NextFunction } from 'express';
import { table } from 'console';

export const initPlayers = (req: Request, _: Response, next: NextFunction) => {
  req.body = {
    ...req.body,
    player1: Player.getFirstInstance(),
    player2: Player.getSecondInstance(),
  };
  next();
};

// Player

export class Player {
  private score: number;
  private static instance1: Player | undefined;
  private static instance2: Player | undefined;
  constructor(score: number = 0) {
    this.score = score;
  }

  incrementPoint() {
    return this.setScore(this.getScore() + 1);
  }

  getScore() {
    return Number(this.score);
  }

  setScore(score: number = 0) {
    this.score = score;

    return this.score;
  }

  resetScore() {
    return this.setScore();
  }

  static getFirstInstance() {
    return (Player.instance1 = Player.instance1 || new Player());
  }

  static getSecondInstance() {
    return (Player.instance2 = Player.instance2 || new Player());
  }
}

Answer 1

Object-Oriented Programming is based on four pillars:

• Abstraction
• Encapsulation
• Inheritance
• Polymorphism

We can improve easily two pillars: abstraction and encapsulation.

Encapsulation

res.render('game', {
   // ...,
   player1Score: game.player1.resetScore(),
   // ...
});

The statement game.player1.resetScore() is called a train wrack. But it does not only look odd, there is even a “law” why it should be avoided: Law of Demeter.

The statement game.player1.resetScore() ignores encapsulation. The object game exposes its internal player1.

Instead of asking for the player to reset the score, we should tell the game to reset the score. This is also known as Tell don't ask: game.resetScore().

To support encapsulation it is common to make the internals of an object private:

class Game {
    constructor(private player1: Player, private player2: Player) {
    }

    // ...
}

Abstraction

if (ruleSet[player1Move.toLocaleLowerCase()].includes(player2Move)) {
// ...    
}

We can abstract ruleSet[player1Move.toLocaleLowerCase()].includes(player2Move). To understand what this statemend is doing, we need to activate our brain cells a little.

A good abstraction helps our brain to understand code much quicker. A abstraced version could look like this:

if (moveOfPlayerOne.isBeating(moveOfPlayerTwo)) {
    // ...
}

Naming

whoWins(player1Move: string, player2Move: string) {
   let result: IGameResult = {
         player1Score: undefined,
         player2Score: undefined,
         message: 'Invalid Choice',
   }

   // ...

   return result
}

When I read whoWins at the first time, I thougt it would return a Player. This whoWins sounds like who will win? Player1 or Player2?

A possible Implementation based on yours

class Game {
    constructor(private player1: Player, private player2: Player) {
    }

    play(moveOfPlayerOne: Move, moveOfPlayerTwo: Move): Result {
        if (moveOfPlayerOne.isBeating(moveOfPlayerTwo)) {
            this.player1.incrementScore()

            return {
                playerOne: this.player1,
                playerTwo: this.player2,
                message: `Player 1 wins with ${moveOfPlayerOne.asString()}`
            }
        }

        if (moveOfPlayerTwo.isBeating(moveOfPlayerOne)) {
            this.player2.incrementScore()

            return {
                playerOne: this.player1,
                playerTwo: this.player2,
                message: `Player 2 wins with ${moveOfPlayerTwo.asString()}`
            }
        }

        return {
            playerOne: this.player1,
            playerTwo: this.player2,
            message: 'Draw'
        }
    }

    // ...
}

I hope this will help you as a little inspiration. From here you can easily improve more. :)