DEV Community

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

Roast the code #1 | Rock Paper Scissors

Keff on May 21, 2022

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 ...
 
nombrekeff profile image
Keff

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

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.

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

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

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

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

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

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?

 
nombrekeff profile image
Keff

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
 
keynabou profile image
Victor Flandin

You should test your code

Collapse
 
nombrekeff profile image
Keff

;) 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

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

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

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

Collapse
 
willypuzzle profile image
Domenico Rizzo

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

Collapse
 
nombrekeff profile image
Keff

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.

 
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

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