4
\$\begingroup\$

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 < match.scoreTeamB;
  var equality = match.scoreTeamA === match.scoreTeamB;

  var betOnTeamA = bet.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?

\$\endgroup\$
4
  • \$\begingroup\$ It could be done in one giant logic statement that would short circuit evaluating subsequent tests as soon as one of them proves true, but I think what you have now likely makes it a lot more readable and this doesn't seem like something where a tiny performance improvement would make any material difference. \$\endgroup\$ – jfriend00 Jul 27 '15 at 19:53
  • \$\begingroup\$ Isn't this just checking which team won (or if it was a tie) and which team the bet betted on winning (or if it bet on a tie)? \$\endgroup\$ – nhgrif Jul 27 '15 at 19:58
  • \$\begingroup\$ That's it @nhgrif \$\endgroup\$ – Yann Bertrand Jul 27 '15 at 19:59
  • \$\begingroup\$ @jfriend00 My apologies for the misunderstanding. \$\endgroup\$ – SirPython Jul 27 '15 at 20:23
3
\$\begingroup\$

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.

\$\endgroup\$
1
  • \$\begingroup\$ Actually, this function is used in a series of other functions to establish a bet "score". Each function reports points on this bet. I will use this one like if(isGoodWinner(match, bet)) { score += 100; } and then make other computations. \$\endgroup\$ – Yann Bertrand Jul 27 '15 at 20:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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