Is my code ok?

Unreal Courses Show
,
#1

Hello,

After struggling a bit in the previous lessons, I actually achieved making the code do what I want on my own and this is the result, which is a bit different from the one that the lecturer did.

image
image1920×1080 129 KB

image
image1920×1080 243 KB

I guess this is not the most readable code, could you please give me some feedback and tell me why it would be better written differently and how.

Thank you very much!
Riccardo

#2

So my first question would be, does your code work? If it works, and I would address the issue of readability, because it would be hard to remember what this code does. Normally to address this issue, I would refactor the code so that OnInput method has very few lines and reads very close to my pseudocode.

#3

Thank you for replying.

It does work, would you change something in particular that I wrote?

I am not too sure of the many if statement that I have created in the else statement (1st picture,line 29).
Would you say that’s wrong doing in terms of writing the code?

#4

I would say way too many lines for one method, I try to aim for 4-5 lines of codes per method*. This tight limit ensure that your functions are doing one thing, and is following the rule of singled responsibility principle from S.O.L.I.D. principles. This functions is setup a new game when game state deems it, check if Input is the correct word than ends the game, if the input isnt the correct word than the function figures out which error message to display, and then check if you have lost the game. Wow that is alot of things for a function to do.
So i would try extracting some of this logic into their own functions so that OnInput responsibility is only calling other methods/functions(yes calling method is a responsibility).

function OnInput(FString input)
{
  If(ReStartGame()) return;
  if(IsInputCorrect(input) return;
  --life; //If the input isn't correct than well we take a life away no matter
  ShowInputErrorMessage(Input);
  SouldEndGame();
}

*sometime you just cant get a method down to 4-5 lines of code.

1 Like
#5

Thank you Raven, that’s very helpful.

I will commit to apply this in my future code even if still a bit hard considering I am still learning and figuring out how to make the code work :sweat_smile: .

Thanks again,
Riccardo

#6

There is a lot to say about personal and team tastes when it comes to readability. your code looks fine. I like when I can look at the code and see the logic in one place. your code is reasonably self documenting too. if I was going to change anything I would reduce nesting levels by returning out of the function block, instead of using the else statement.

if(GameOver)
{ 
    ClearScreen();
    SetupGame();
    return;
}
if(Input == HiddenWord)
{
    //...
    return;
}
if(Input != HiddenWord)
{
    //...
}
if(Input.Len() != HiddenWord.Len())
{
    //...
}
if(Lives < 1)
{
    //...
}
//etc...

notice how nothing is nested. it can be hard to follow nesting because there are many things that can nest, and else statements can be hard to follow at a glance.

#7

Here is a good link about refactoring and what make code clean or dirt.
https://refactoring.guru/refactoring

Privacy & Terms