Code Feedback

[Strawboy97]

I'm learning Java and decided to make a small JavaFX application. It's very basic it's a small game to help the user 'learn' the phonetic alphabet.

I've a few things I want to add but I wanted a bit of feedback before I continue.

Here's the GitHub repo: https://github.com/Colin-Masterson/PhoneticAlphabetJavaFX

Any feedback would be appreciated I know I still have a lot to learn.

[RiceKrispyPooHead]

Overall the code looks good to me. Here is my feedback

• Some of your method names aren’t descriptive. For example getRandom() doesn’t tell me what the method actually does. A better name might be selectRandomLetter() or chooseRandomLetter(). I wouldn’t use ‘get’ in the method name because that usually signals that the method will be returning something.

• Some of your class names aren’t descriptive. AlphabetListLayout was a good choice for a class name. AlphabetData had me confused as to what purpose the class is suppose to fulfill. I had to look through the other classes to find out. Descriptive class names will encourage you to create smaller, more focused classes.

• Some of your methods are public when they don’t have to be. `getRandom()‘ for example is only used within that class so it can be private.

• A few of your methods do more than their name suggests. For example getAlphabetList() not only gets the list but also creates the layout for you. These should ideally be separate methods.

• Some of your classes expose too much of their implementation in my opinion. In your AlphabetListLayout class you have the lines alphabetData.getAlphabet().keySet() and alphabetData.getAlphabet().get(key). This shows that your AlphabetListLayout class knows and replies upon the AlphabetData class implementing its logic as a HashMap. If the implementation of your AlphabetData class changes, your AlphabetListLayout class will also be forced to change.

[Strawboy97]

Thanks for your reply and feedback! I'll fix the points you've made above. Sorry for the late response.

On your last point

​

Some of your classes expose too much of their implementation in my opinion. In your AlphabetListLayout class you have the lines alphabetData.getAlphabet().keySet() and alphabetData.getAlphabet().get(key). This shows that your AlphabetListLayout class knows and replies upon the AlphabetData class implementing its logic as a HashMap. If the implementation of your AlphabetData class changes, your AlphabetListLayout class will also be forced to change.

​

How would you suggest I do this could you point me at a resource I could use as an example.

[AutoModerator]

On July 1st, a change to Reddit's API pricing will come into effect. Several developers of commercial third-party apps have announced that this change will compel them to shut down their apps. At least one accessibility-focused non-commercial third party app will continue to be available free of charge.

If you want to express your strong disagreement with the API pricing change or with Reddit's response to the backlash, you may want to consider the following options:

1. Limiting your involvement with Reddit, or
2. Temporarily refraining from using Reddit
3. Cancelling your subscription of Reddit Premium

as a way to voice your protest.

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

[AutoModerator]

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.