DEV Community

Cover image for Roast the code #1 | Rock Paper Scissors
Keff
Keff

Posted on

Roast the code #1 | Rock Paper Scissors

Hey there! 👋

I'm starting a new series called "Roast the Code", where I will share some code, and let YOU roast and improve it.

There's not much more to it, just be polite and constructive, this is an exercise so we can all learn together.

Now then, head over to the repo and roast as hard as you can!!

GitHub logo nombrekeff / rock-paper-scissors

Little rock paper scissors code example, for a post on DEV

Rock Paper Scissors | Roast My Code

Hey there! 👋 This repository is an implementation of the Rock Paper Scissors game.

My goal with this code is to publish a post over on DEV.to and let people roast it, and improve it if they feel like it!

Getting Started

  1. Clone repo
  2. Change diretory to the cloned repo
  3. Install deps npm install
  4. Run the game npm start
  5. Code starts at main.ts
  6. Roast the code!

How to roast?

Head over to DEV.to, and leave a coment on the post. If you want to, you can also send in a PR with your proposed changes here!




If you want your code to be roasted on the series, please contact me in the comments or send my a PM here on DEV. And I will share it in a future instalment of the series!

Discussion (31)

Collapse
simeg profile image
Simon Egersand 🎈

Cool idea!

From looking at the code I can tell you're an experienced programmer. You have abstractions and they make sense. It also looks easily testable with the interfaces in place. Nice job! 🤟

It would be interesting to review someone with less experience so hopefully someone is brave enough to reach out 🙂

Collapse
nombrekeff profile image
Keff Author

Cheers! That's always nice to hear!!!

There's a couple of places where I'm not 100% happy with it though, like the GameUi interface and the limitation of just having 2 players. But with a bit of thought it could be easily improved

Yup, I'm hopping for that too!! I really think these kinds of exercises can be highly beneficial for less experienced people.

Collapse
nombrekeff profile image
Keff Author

Fair enough, and I totally agree!

I have no excuse, but If I'm honest safety is something I take quite seriously on real projects, this one was more of a fun project. I did it a while back and after reading your comment I remember that I left wanting to work on that part. Might need to consider the order in which I do these things though!

The complexity part was kind of intentional, I made a really simple version first, and I then decided to do it in that way as to make it more extensible. My idea was to allow for rules, UI's and AI's to be built without much need to change the code. For example, having different levels of AI, ranging from simple (random picks) to more complex AI's, maybe using machine learning and so on. If I had taken a bit more time I might've come up with something simpler, but I got bored 😅

What parts would you say are overly complex?

Would you mind elaborating on what you mean by hard to test? And how would you say it could be improved?

Collapse
willypuzzle profile image
Domenico Rizzo

I think generalization, interfaces and abstract classes are always a good idea. The project it's not perfect I agree but you put the basics to further developments

Thread Thread
nombrekeff profile image
Keff Author

Yeah, it was never intended to be perfect, otherwise the whole point of the post would be lost! I don't know if there is such think as perfect code to be honest, code evolves over time and improves continuously, so I hope more people like you come on board and help improve it even more, so we can all learn and improve together! Thanks again for the PR

nombrekeff profile image
Keff Author

Cool stuff, I did a similar solution before the one shared in the post, but in plain js

I'll roast you a bit, but I don't have much to add though. I'd say importing the whole lodash library just to use the isEqualWith is a bit overkill for this simple scenario (quite ironic by my part to say that though xD).

And yeah enums are kinda weird in TS, but I sometimes miss some of the features they have in other languages.

nombrekeff profile image
Keff Author

Makes sense, in this case it might've been interesting to create a tuple type for the rules/conditions, and then just write a function to compare them, as you know they will only have 2 values (at least for the example provided) something like:

const areTuplesEqual = (t1: Tuple, t2: Tuple) => {
    return t1[0] === t2[0] && t1[1] === t2[1];
}
Enter fullscreen mode Exit fullscreen mode

I'm glad you're enjoying it! It's cool that you could take something away from it too!

nombrekeff profile image
Keff Author

It was quite clear, not to say it's all correct or that one should follow the advice in all scenarioos. But I apreciate that you took the time and effort to elaborate so much. I'll take a couple of things from it. One thing to consider though is that the project took 2-3 hours to build, that's something important to note as welll xD Might've been interesting to explain in the post description...

Collapse
jordanjlatimer profile image
jordanjlatimer

I really like your code! It’s easy to read and follow what’s going on.

Honestly, the only thing I can see that you could improve are kinda nit picky, but hey, at least I have something for you.

In your abstract GameUi class, all of your methods are verbs except for “initialMessage”, which is a noun. I would change that to “displayInitialMessage” or some over verb-like name for consistency.

Same with the “whoWins” method in Ruleset class, and the “divider” method in your CliUi class.

The “isWinner” method of the Rule class feels weird to me. Maybe because it doesn’t make sense for a rule to be a “winner”? I don’t know, it could just be me.

Anyway, great work!

Collapse
nombrekeff profile image
Keff Author

Thanks for the feedback and the kind words!

In your abstract GameUi class, all of your methods are verbs except for “initialMessage”, which is a noun. I would change that to “displayInitialMessage” or some over verb-like name for consistency.

I really like this suggestion, I'm all in for this kind of changes!

The “isWinner” method of the Rule class feels weird to me. Maybe because it doesn’t make sense for a rule to be a “winner”? I don’t know, it could just be me.

I totally agree, funnily enough I had a really difficult time finding a better name for the "isWinner" method, so I decided to let that one slip until I found a better name. Do you have any suggestions?

Collapse
keynabou profile image
Victor Flandin

You should test your code

Collapse
nombrekeff profile image
Keff Author

;) I knew this would be one of the first comments 🤣

Fair enough, I did not add tests in this particular case as I do tests every day at work and this little game was just a way for me to relax... But I do agree, highly. Maybe someone will be up to add more tests!

Collapse
keynabou profile image
Victor Flandin

It was an easy one 😁

Collapse
dewaldels profile image
Dewald Els

Some nicely written code.

Possible typo in /impl/game.ts on line 34: this.cpuName = 'cup';
Guessing that should be cpu?

Collapse
nombrekeff profile image
Keff Author

Good catch!!!

Collapse
paratron profile image
Christian Engel

Isn't that what happens all the time when someone posts some code on the internet? 🙈

Collapse
nombrekeff profile image
Keff Author

I suppose it is 🤣 Though in a kind and respectful community, not the same as posting on reddit though xD

Collapse
willypuzzle profile image
Domenico Rizzo

I made a pull request to correct the unhandled wrong user input

Collapse
nombrekeff profile image
Keff Author

Cheers, I just added a little review!!! Quite a nice simple fix!

Collapse
martha220 profile image
martha220

Its good and very easy when try to practice myself.

Collapse
willypuzzle profile image
Domenico Rizzo

You wrote very nice code. I'll try to do some improving if I have time

willypuzzle profile image
Domenico Rizzo

Everyone has his/her own style of programming. What I said is that he just put a good basic for a further development. Of course the project is not perfect (it's just a project done for fun). But I think he put a reasonably level of abstraction in order to make things improvable.

Thread Thread
nombrekeff profile image
Keff Author

Yup, maybe it was not a good idea after all 🤣

Nah, it was kind of helpful, I can take a couple of things from this, cheers! Thanks for taking the time to expand!

Thread Thread
willypuzzle profile image
Domenico Rizzo

Every step is a step toward knowledge