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 ...
For further actions, you may consider blocking this person and/or reporting abuse
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 🙂
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.
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?
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
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
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
isEqualWithis 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.
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:
I'm glad you're enjoying it! It's cool that you could take something away from it too!
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...
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!
Thanks for the feedback and the kind words!
I really like this suggestion, I'm all in for this kind of changes!
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?
You should test your code
;) 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!
It was an easy one 😁
Some nicely written code.
Possible typo in
/impl/game.tson line 34:this.cpuName = 'cup';Guessing that should be cpu?
Good catch!!!
Isn't that what happens all the time when someone posts some code on the internet? 🙈
I suppose it is 🤣 Though in a kind and respectful community, not the same as posting on reddit though xD
I made a pull request to correct the unhandled wrong user input
Cheers, I just added a little review!!! Quite a nice simple fix!
Its good and very easy when try to practice myself.
You wrote very nice code. I'll try to do some improving if I have time
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.
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!
Every step is a step toward knowledge