+ 1
[code-review][java] Grocery Cost List
I'm a beginner in Java and want to improve and learn. Therefore I'd appreciate if an intermediate/expert Java developer could look over my code and review it. I'd be grateful for any improvement suggestions or feedback. Thanks in advance. :) This is a grocery cost list: By typing in certain items you want to buy, e.g. fish, egg, etc., the program calculates how much you have to pay for the items in total: https://github.com/thadeuszlay/17_groceryList
3 Réponses
+ 6
This was really well done, good use of map/Hashmap!
I think you can reduce some things you don't need:
Line 27
You never actually use the boolean elsewhere, so you could just write:
calculateUserInput(true);
So you just pass true through the function (basically a default value).
If you preferred having the boolean there for readability that's fine too.
Line 38/39
calculateUserInput(true);
break;
It would make more sense to use continue; Instead of re-calling the function when it's just going back to the top of the loop with the same condition result.
If you do the above ^^^ you can then remove the if statement on:
Line 48
You can put getResult() after the loop, instead of inside the if statement. The loop is already determining that condition and you don't seem to want to do anything after the result inside the loop anyway. (The if statement becomes redundant).
------
I actually think the resultList isn't necessary. Unless you plan on showing both what the user bought and the price of the items purchased.
Currently, the list could be replaced by a single double.
double totalPrice;
Then you also wouldn't need to use that for loop in the 'getResult' method.
However again, this is only better if you don't want to show what the user purchased and the price of each item. If you only wanted to show the price of each item purchased, it may be better to use an array. Same thing with if you only wanted to show each item.
(So: Hashmap for both item and price, array for one, double for neither).
------
*"Do you want more items (y/n)"
"(y/n)", but really it's more like (y)?. If I type anything other than 'n' the program takes it as an 'n' anyway.
Great job though! Not much to look over here tbh. Some of what I said might be a bit picky :P.
If this response was a bit late, I blame the LikeStorm thread for the notification bombs. sorry hehe.
+ 1
@Rrestoring faith: I took your feedback into consideration in this implementation:
https://www.sololearn.com/Discuss/369226/reached-lvl-13-today-includes-code
Please have a look and tell me what you I can improve. Thanks in advance. :)
0
you can be as nitpicking as you want. the more the better. I highly appreciate.