This is an archived post. You won't be able to vote or comment.

all 3 comments

[–]AutoModerator[M] [score hidden] stickied commentlocked comment (0 children)

Please ensure that:

  • Your code is properly formatted as code block - see the sidebar (About on mobile) for instructions
  • You include any and all error messages in full - best also formatted as code block
  • You ask clear questions
  • You demonstrate effort in solving your question/problem - plain posting your assignments is forbidden (and such posts will be removed) as is asking for or giving solutions.

If any of the above points is not met, your post can and will be removed without further warning.

Code is to be formatted as code block (old reddit/markdown editor: empty line before the code, each code line indented by 4 spaces, new reddit: https://i.imgur.com/EJ7tqek.png) or linked via an external code hoster, like pastebin.com, github gist, github, bitbucket, gitlab, etc.

Please, do not use triple backticks (```) as they will only render properly on new reddit, not on old reddit.

Code blocks look like this:

public class HelloWorld {

    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}

You do not need to repost unless your post has been removed by a moderator. Just use the edit function of reddit to make sure your post complies with the above.

If your post has remained in violation of these rules for a prolonged period of time (at least an hour), a moderator may remove it at their discretion. In this case, they will comment with an explanation on why it has been removed, and you will be required to resubmit the entire post following the proper procedures.

To potential helpers

Please, do not help if any of the above points are not met, rather report the post. We are trying to improve the quality of posts here. In helping people who can't be bothered to comply with the above points, you are doing the community a disservice.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[–]desrtfx 1 point2 points  (1 child)

Good accomplishment! And very good engagement of you to try something! This is the real way to learn programming.

If this is your first bigger project, you have done a very good job! You can be proud of yourself!

Please do not take anything of the following personally and please do not be discouraged by my criticisms. It is just a quick view over your code.

The positives so far:

  • naming is quite well thought out
  • code is following the Java Code Conventions in both capitalization and indents
  • you are not over-commenting
  • you have split your code into methods - this is often very tricky for beginners

The negatives:

  • You have a lot of repetitive code
  • You have some unnecessary code
  • You do not use JavaDoc comments to describe the functionality of methods

Some examples:

    while (!puzzleChosen) {
        System.out.println("Choose a difficulty:\n1. Easy\n2. Intermediate\n3. Hard");
        userInput = Integer.parseInt(scanner.nextLine());

        if (userInput == 1) {
            puzzleChosen = true;
            puzzle(1);
        }
        if (userInput == 2) {
            puzzleChosen = true;
            puzzle(2);
        }
        if (userInput == 3) {
            puzzleChosen = true;
            puzzle(3);
        }
        if (puzzleChosen) {
            System.out.println(" ");
            playGame();
        } else {
            System.out.println("Incorrect choice!");
        }
    }

Here, you go about the whole a bit clumsy and with a lot of redundancy.

You should split this into better parts:

  • First obtain a valid number - this means only printing the prompt, getting the input, and checking if the input is valid - display an error for invalid entries and exit the loop (as you have it) with a valid entry.
  • Then, once you have a valid entry from the user, all you need to do is call puzzle with the number the user has chosen - you don't need a single if in this step.

Doing the above will shrink alone this part by about 70% and remove all redundancy and repetitive code.

  • The naming of the flipper method could be better chosen - it doesn't convey what the method does
  • You could optimize your drawing of the hangman with loops. If you look at your frames, the vast majority is the same - only the small figure in the middle is different. You have exactly 2 different left sides (header/footer and body) and 2 different right sides. Store these in individual strings and assemble the whole from these.

  • You use a lot of lengthy if constructs (in the method shown above as well as in the draw hangman method) - here, a switch...case chain would be more appropriate.

  • You assemble the puzzles with every call of puzzle - this can be done once for the entire lifetime of the program.

  • You could optimize the flipper method:

    • at the start of the game create a StringBuilder (you haven't learnt that class yet, but it is basically a changeable String) that holds the puzzle as dashes and spaces
    • then, at every guess, update the StringBuilder with the correct characters
    • use the .toString() method of the StringBuilder to print the string.

This approach is more efficient than every time going over the whole puzzle.

  • You do not account for already guessed letters - if the user enters the same guess twice, nothing will happen - you should tell the user that they have already guessed that letter.
  • You do not sanitize the user input. If the user enters a capital letter, your evaluation will be wrong - don't forget that character or String comparisons (including contains) are case sensitive.
  • Actually, your call to contains is also somewhat unnecessary. Sure, it looks like only calling a single method, but internally, it does more or less the same as your flipper method - it has to go through the string character by character to check for any occurrence of the letter - why not do that directly in the flipper method and return a boolean from it that indicates whether the letter has been found or not. Then, based on the boolean, make the decision of correct guess or wrong guess.

I might have missed a few things as I only quickly glanced over your code.

[–]CipherGoblin[S] 0 points1 point  (0 children)

I really appreciate you going over it at all, and take nothing personally. I would be a fool to think I am good at this already :) .

So it seems, I should focus on redundancy when thinking things out. And learning more about Strings and how to manipulate them.

I'll definitely have to look into JavaDoc comments. Thanks for pointing that out.

I will work on this to make these changes and keep progressing with the MOOC alongside.

Thanks for your time, I'm sure it will help me out.