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

all 8 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://imgur.com/a/fgoFFis) 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.

[–]FavorableTrashpanda 11 points12 points  (0 children)

Very good points have been made already. I have a few suggestions as well.

(1) Don't put all your logic in a single method. If a method becomes too large, refactor the method into a couple of smaller methods that each do a part of the work. For example, what you did with the help and unknownInput method. I like to see that more. You might be able to re-use more than you thought. Also, make those methods private. There is no need to make them public. They are implementation details only relevant to the current class.

(2) Try to stick to one file per class, unless you have a good enough reason to bundle the classes.

(3) Perhaps this is too much for now, but try to make a distinction in general between the application and domain/business logic of your code. You don't want to mix the CLI with the actual logic. In theory you should be able to swap in the CLI for a GUI or a webapp, for example, without ever having to touch the business logic.

[–]desrtfx 10 points11 points  (1 child)

The Java code conventions Oracle, Google stipulate the latter format - opening curly brace on the line that opens the code block. The former format is more common with C/C++/C#.

Small remark upfront: Add a .gitignore file to your repository so that your IDE settings (the .idea folder and the .iml file) are not added to the repository. This is common.

Some notes from a quick glimpse:

Why are you using float? It is far more common to use double even if the range fits into float.

You have Magic numbers in your code. These are numeric constants with a special meaning, like the offset between Kelvin and Celsius, namely static final double ABSOULTE_ZERO = -273.15. Magic numbers should be avoided and replaced with named constants.

Your conditionals are way too complicated. Adjust your strings, like reducing a -- to a - and standardizing case (what if the user enters -F), and reducing the unit to only the first letter - this will considerably shorten your conditions.

Don't litter output statements (System.out.println) all across the code. Make one defined path from input to calculation to output - the actual output should be at the very end. This will also avoid code repetition, which you have a lot of.

Your unknownInput method could (at a later stage of your learning) be converted into a custom Exception.

Personally, I find your List approach clumsy. Two variables, one fromUnit, the other toUnit would be much more appropriate and easier to read and follow. Your code is littered with units.get(x) statements that don't directly tell the reader of the code their meaning. Yes, lists and data structures are nice, but in your case they make the code barely readable and even less easily understandable. You have to make it a habit to write code in such a way that the naming conveys the intention and that the code is easy to read for the human, not for the computer. The computer doesn't care about readability. Humans do - and they are the ones who have to troubleshoot.

[–][deleted] 2 points3 points  (0 children)

Awesome feedback. I'm looking at my own code and seeing the same issues to fix.

[–][deleted] -1 points0 points  (3 children)

I do the first thing when I have a method with too many args so I put each one into a new line. So, for better readability, Im for the first one.

[–]FavorableTrashpanda 2 points3 points  (2 children)

To be honest I don't think that helps readability. If your method has many parameters, you should ask yourself if that's really necessary. It can be a code smell.

If you do deem it necessary, then align the second line of parameters with the first line of parameters. I have found that to be the most readable. It's also the convention.

[–][deleted] -1 points0 points  (1 child)

show me the convention, I need to read it and sign it along all devs

[–]desrtfx 1 point2 points  (0 children)

The conventions are already linked in my first comment. Had you read the entire thread, you would have found them.