HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Function to check that two objects have "equivalent" values

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
equivalentobjectsfunctiontwothatvaluescheckhave

Problem

I have two objects, a bet and a match. A match has 2 teams and both teams get a score at the end of the match. A bet has two scores set up before the match begins. Here I need to know if the bet specified the same winner as it's set on the match (once it's done).

// isGoodWinner
//
// @description :: return true if has bet on the good winner (or equality)
// @param       :: match (required): the match instance
//                 bet (require): the bet concerned
function isGoodWinner(match, bet) {
  var teamAWins = match.scoreTeamA > match.scoreTeamB;
  var teamBWins = match.scoreTeamA  bet.scoreTeamB;
  var betOnTeamB = bet.scoreTeamA < bet.scoreTeamB;
  var betEquality = bet.scoreTeamA === bet.scoreTeamB;

  return ((teamAWins && betOnTeamA) ||
        (equality && betEquality) ||
        (teamBWins && betOnTeamB));
}


I think my naming is poor and I don't know how to do it easier, do you have any advice?

Another question I have is: how can I be sure to unit test each case?

Solution

Personally, I think your naming is just fine. However, the only thing I'd change is equality. That name is very general.

Luckily, since we are talking about races, we can use information about races to name the variables. As you probably know, in a race, when two or more teams end with the same exact score, it is called a tie. Therefore, you should rename that variable to tie.

And, now that I think about it, your function could use some renaming too. isGoodWinner doesn't really tell you what the function is doing; what is a "good winner"? I would name it to isWinningBet. That name tells you if the bet (not the winner) was on the winning team.

You have a few extra variables here. The ones in question are teamAWins and teamBWins, and betOnTeamA and betOnTeamB.

These can be simplified. If someone did not be on team A, they bet on team B (ties don't matter because the variable holding the tie will handle that). And, if team A did not win, then team B win. Both of these apply visa-versa, too.

That being said, we can reduce the variables you need:

var teamAWins = match.scoreTeamA > match.scoreTeamB;
var equality = match.scoreTeamA === match.scoreTeamB;

var betOnTeamA = bet.scoreTeamA > bet.scoreTeamB;
var betEquality = bet.scoreTeamA === bet.scoreTeamB;


Good job writing documentation for your function. I don't often see JavaScript code with documentation, even thought it should.

And, good job using the === operator. Most of the time, people will use the == operator in JavaScript which is a bad idea on a few levels.

Your design is a little confusing.

Judging by these variables:

var betOnTeamA = bet.scoreTeamA > bet.scoreTeamB;
var betOnTeamB = bet.scoreTeamA < bet.scoreTeamB;
var betEquality = bet.scoreTeamA === bet.scoreTeamB;


a person can bet different amounts on both teams. However, your function declares that, if they bet more on, say, team A, then their full bet is on team A.

Now, I don't know what the rest of your code is supposed to do, but I assume you are going to have some payback for winning bets. Well, if a user bets (for example) 10 on team A and 5 on team B, then team B wins, how are you supposed to know which amount to payback?

You wouldn't want to payback based on a 15 bet because the person bet 15 in total, you'd want to payback based on a 5 bet because that bet was on the winning team.

There are two ways you could fix this:

-
Have the function return an object, saying if the bet was on a winning team and if so, which team the bet was on.

-
Change your bet design so a bet can only be placed on a single team.

Personally, I'd go with the second option. This "aligns" your objects more, so one object isn't doing the work or holding the information of another. And, it makes more sense in real life: at a real race, you could place a single bet that applied to multiple teams; you place multiple bets, one for each team.

Code Snippets

var teamAWins = match.scoreTeamA > match.scoreTeamB;
var equality = match.scoreTeamA === match.scoreTeamB;

var betOnTeamA = bet.scoreTeamA > bet.scoreTeamB;
var betEquality = bet.scoreTeamA === bet.scoreTeamB;
var betOnTeamA = bet.scoreTeamA > bet.scoreTeamB;
var betOnTeamB = bet.scoreTeamA < bet.scoreTeamB;
var betEquality = bet.scoreTeamA === bet.scoreTeamB;

Context

StackExchange Code Review Q#98250, answer score: 3

Revisions (0)

No revisions yet.