New answers tagged

1

You didn't include NodeRepo so I'm assuming you're working with JPQL. Reduce redundancy One of the first things I was taught in database design was to avoid redundant data. Packing the first/last status to each node is entirely redundant and can easily lead to situations where your queue has more than one head or tail. You would also need an index on both ...


2

public static double returnLogisticEquation(double x, double r) { The name is not quite correct, as it does return the result of equation, not the equation itself, so more like calculateLogisticEquation. public static double returnLogisticEquation(double x, double r) { return r * x * (1 - x); } I'm torn on this one. Normally, I say that ...


2

This program is used to create a random bank card number. Card number is created using account type and client location for the first eight digits and the remaining 8 digits are completely random. This isn't a critique of the coding per se, but of the requirements gathering. Bank cards do not allow for a fully random last eight characters. Because the ...


3

Always use the base class / interface in the left part of the variable when possible By setting the java.util.List interface in the variable part, this could make the code more refactorable in the future, since you could easily change the list type without changing everything (inheritance). Before private static ArrayList<String> storedCardNumbers = ...


4

Hi Nicholas and welcome to code review. A few things that I noticed in your code: The indentation isn't consistent. The closing bracket of menu isn't where it should be. The whole setAccountLocation method is one tab to far right. The readability of the code would be better if you'd fix that. In Eclipse, you can press Ctrl + Shift + F to automatically fix ...


3

Few suggestions: The class Gate is final, therefore all its methods are implicitly final, no need to specify the modifier on each method. Docs. The name Gate is a bit confusing because that class represents an input or output. One variable per declaration: private boolean value = false, force = false, forceValue = true; is not considered good practice. You ...


1

In addition to your specific question on the isValid() methods, a few hints on code style, as this is the CodeReview site. Your solve() method is well done regarding its recursive calls, but it mixes computation and user interface (output) when it finally found the solution. In professional development, we separate these concerns as much as possible, e.g. to ...


2

One approach to unify all the various validity tests is to introduce what I'd call "groups": Each row is a group. Each column is a group. Each of the 3*3 blocks is a group. Each group consists of 9 cells, and can be represented by a list of 9 coordinates. You can create the 27 groups either by manually writing down the coordinate lists (tedious ...


4

I realise that this isn't as much of a review as a rewrite, but I still think it's on topic as an answer on this site as sometimes the right answer to your problem is to take a completely different approach that avoids the problems in the original attempt by design rather than tweaks. I wrote a sudoku solver a long time ago. One of the things I did to reduce ...


7

To remove duplication, // check block 1 if (row >= 0 && col >= 0 && row <= 2 && col <= 2){ if(!isBlockValid(n, row, col, board, 0, 2, 0, 2)){ return false; } } you can rewrite these nine sections as this single block final int blockRow = 3 * (row / 3); final int blockCol = 3 * (col / 3)...


6

public static void load_Directory_Files(File directory) { File[] fileList = directory.listFiles(); if(fileList != null && fileList.length > 0){ for (int i=0; i<fileList.length; i++){ if(fileList[i].isDirectory()){ load_Directory_Files(fileList[i]); } else { String name = ...


1

In this answer, you are told that you can precompute the list of primes. But if space is not a concern, we can go further and precalculate the sums. Assuming we have an array prime such that prime[x] tells us if x is prime or not. long[] primeSumUpTo = new long[prime.length]; // this explicit initialization is not necessary as 0 is the default value ...


2

Example for an IndexColorModel. /* START */ public static final class GUI { public static final Color BLACK = new Color(0x00, 0x00, 0x00); public static final Color MAROON = new Color(0x80, 0x00, 0x00); public static final Color GREEN = new Color(0x00, 0x80, 0x00); public static final Color OLIVE = new Color(0x80, 0x80, 0x00); ...


5

Protocol Well, first the good news. I don't see anything particularly wrong with the algorithms or parameters used. Class design This is a badly designed class with a lot of copy / paste going on (and I've found a clear indication for that at the end, where you copy code directly from this site). I'm not a big fan of static classes, and this one doesn't lend ...


2

You could either use the real Java console, but that would mean you have to recreate your entire application. If you really insist to use Swing, I would do it in another way. Create some custom print() methode for printing ASCII-symbols into a buffered-image, scale this image and use it as background for just a single AWT/Swing component. After that you ...


2

For me, the best production code is the code that is concise and simple, especially with respect to the amount of branching. The following code slightly changes the semantics (in a good way, I think) and unifies the multiplication: private double getTotalPrice(int quantity, double itemPrice) { // Charge for 13 as if it were only 12 if (quantity == 13)...


6

Nice implementation and refactoring, few suggestions: AbstractCommand class I created a base class for all subcommands, AbstractCommand. I think a better name is SubCommand. The type of the class doesn't need to be in the name. The instance variable commandName can be shorten to name, since it's implicit that it's the subcommand name. The second argument ...


4

Your formatting is partly off, use a code-formatter. public class Main extends JavaPlugin { I'm not familiar anymore with the Bukkit naming convetions, but this should most likely be called SpigotPlugin. Main is kinda reserved for main classes which are the main entry point for applications. Do you require the static instance mechanic? If you can, you ...


0

Style Guide Please read a style guide. You should always use braces when using a loop or if-statements. add access modifiers make spaces between operators. Don't use *-imports. import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStreamWriter; import java.io....


4

What I am most interested in is how to name the magic numbers. Alternative opinion - none of the numbers should be in code in the first place. Every answer with named constants is just wrong. They should all be in data. Assumptions: pricing rules are all predicated only on quantity pricing rules change more frequently than business logic if you buy 26 of ...


2

One additional suggestion regarding the .95 constant: In sales, it's more natural to express the discount percents instead of the factor private static final double QUANTITY_DISCOUNT_PERCENT = 5.0; private static final double QUANTITY_DISCOUNT_FACTOR = 0.01 * (100.0 - QUANTITY_DISCOUNT_PERCENT); ... return itemPrice * quantity * QUANTITY_DISCOUNT_FACTOR; (...


6

There is no need to convert String to char[] array. Individual characters are accessible via String.charAt method. There is also no need to explicitly spell out the hexadecimals. Consider Character.digit(message.charAt[i+1], 16) != -1.


8

Welcome to Code Review. Your code returns false for the message "hello&9", because the method doesn't check the last character. Change the condition in the for-loop to message.length() - 1. An alternative is to use a regular expression: public static boolean isMessageColoured(String message) { return message.matches(".*&[a-f0-9].*...


6

Another possibility would be an object-oriented approach in which we use the concept of abstraction. For this we could abstract the logic quantity == 13 as a method with the sigiture isOneOffDiscountFor(int quantity): private boolean isOneOffDiscountFor(int quantity) { return quantity == 13; } private boolean isCheaperDiscountFor(int quantity) { ...


2

private Object ensureType(Class<?> expectedClass, Object value) { Your names could be slightly better, given that this function does not only ensure something, but also converts it. private Object convert(Class<?> targetClass, Object value) { throw new ClassCastException( "argument " + value + " of type " + ...


5

Other points: This is Java, so it's important that your method be marked static given that it is forced to be a class member Make a temporary variable double subTotal = itemPrice * quantity; to avoid duplication of the multiplication on the bottom When you do factor out magic numbers, whether they belong as class members or locals depends on the rest of the ...


8

Refactoring The first thing I'd do is remove the redundant else statements. If you return from every branch, then there's really no need for the else clauses, they're just adding noise. Constants? It seems like there's some scope for constants / or lookups since there's some numbers floating around that seem to have meaning (discount amount, discount ...


0

Streams A couple things to keep in mind. The new syntax for .forEach() is cool but it should be used for simple things and it should not be nested. Either refactor it to avoid nesting or extract the nested parts to methods. This way those parts will have names which should make their purpose obvious. Don't modify collections with .forEach. If you need to ...


5

Nice implementation, it's already efficient but as noted in the comments can be improved. Few general suggestions: Format the code: many IDE support auto formatting or you can use an online formatter. Omitting brackets is not good practice: it's better to include the brackets to avoid confusion when reading or changing the code. See the difference from: ...


3

PhoneService class In my opinion, the PhoneService should be converted to an interface exposing the methods. public interface PhoneService { void sendNotificationByPhoneNumbers(String title, String message, List<String> phoneNumbers); } Then, the PhoneServiceImpl class can implement the interface PhoneService. @Slf4j @Service public class ...


9

TL;DR private String increment(String input){ char[] chars = input.toCharArray(); boolean stopLoop = false; for(int i = chars.length - 1; i >= 0; i--){ char character = chars[i]; switch (character) { case '0', '1', '2', '3', '4', '5', '6', '7', '8' -> { chars[i] = (char) (character + 1); ...


3

Disclaimer: Not a Code Reviewer Just tested your code, it has a 1ms runtime with 99.97% faster than other solutions. Don't pay too much attention to these runtime/memory data provided by LeetCode especially when: it comes to memory; or test cases are too limited (such as Teemo) because their benchmarkings are not so accurate. Maybe instead we'd format ...


3

Extract some of the logic to methods. As suggested by @bobby, the code in the Increment#increment method can be extracted in more than one method. I suggest that you extract the logic into multiples new methods; this will make the code shorter, easier to read and make the code more testable in any test framework (you will be able to test each new method with ...


8

Because this is an interview question, the most important part is the questions you ask. The questions show that you understand that requirements are seldom complete and know what kind of limitations affect the implementation of the algorithm. The ones that need to be asked here are: How long are the input strings of typical input? How long are the numeric ...


5

static String padLeftZeros(String inputString, int length) { Having no modifier specified means that it is package-private. Personally, I like to pretend that package-private does not exist, as it easily leads to "hidden" coupling between classes, and makes it harder for extending classes to actually utilize the classes. On top of that, you can ...


3

String s = "Even aside from the rain and wind it hadn't been a happy practice session. Fred and George, who had been spying on the Slytherin team, had seen for themselves the speed of those new Nimbus Two Thousand and Ones. They reported that the Slytherin team was no more than seven greenish blurs, shooting through the air like missiles."; s is ...


1

Scanner keyboardInput = new Scanner(System.in); You call this on each recursive call to getSelection. That's unnecessary. You could simply create one Scanner and use it for the entire program. That could be as simple as adding a class variable: private static Scanner keyboardInput = new Scanner(System.in); Or you could use a try-with-resources and ...


4

Defualt Java style is to have the opening braces on the same line, but that's down to preferences. Just be consistent in your code. private static final String SETTINGS_FILE_PATH = "D:/Programing/Java/Software Installer/src/main/resources/Settings.ini"; You want to pass that path, somehow, as it is not portable. public class SettingsManager { ...


5

1. Proper indentation Please have a look at a style guide. One important topic of style guides is how to "correctly" indent your code: import java.io.File; import java.util.Scanner; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.util.InputMismatchException; public class WordGames { public static void main(...


2

getSelection is a recursive method that also terminate the program. There is no need to return anything. All the code behind that call in your main method will never be executed and is useless.


4

Do you have considered to use only one stack ? You can wrap your numbers into a constant entry that returns the number. And create one operation entry for each valid operation. So that you can "just" reduce your stack until she has one item. Stack<Function<Integer, Integer>> operations = new Stack<>(); public Integer resolve(...


3

You should use an automatic formatter like suggested in a comment, or an online one such as this https://www.tutorialspoint.com/online_java_formatter.htm or https://codebeautify.org/javaviewer Some changes that we get are Before switch(selection) { case "1": subStringProblem(); break; case "2": pointsProblem(); ...


5

One additional remark on validate(). You have three output cases: invalid number operator Instead of encoding them as integers (which is not a natural natural choice, as you can't meaningfully add, subtract or multiply them), I'd introduce an enum: enum Validation { INVALID, NUMBER, OPERATOR } Then the validate() method reads public Validation validate(...


5

Consider using the Deque instead of the Stack As stated in the documentation, the use of the Deque is preferred. You can have more information on SO. Replace the for loop with an enhanced 'for' loop In your code, you don’t actually need the index provided by the loop, you can the enhanced version. Before for (int i = 0; i < operArray.length; i++) { //[....


3

Nice implementation, it's already efficient but I noticed something that can be improved, find my suggestion below. Time complexity public String processStudents(){ //... // O(s) where s is the number of students for (Student student : StudentsList){ // O(d*log(d)) d is the number of dates of the ...


2

I noticed you wrote the following method: private LocalDate convertToLocalDate(String date){ DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-d"); return LocalDate.parse(date, formatter); } You are instantianting a new DateTimeFormatter object everytime you calls this method and this is expensive, you could create one ...


3

I would suggest using the groupingBy collector. public static void main(String[] args) { int[][] arr = {{ 1, 2, 3 }, { 2, 3, 4 }, { 2, 4, 0 }}; IntStream.range(0, arr.length) .boxed() .collect(groupingBy(i->Arrays.stream(arr[i]).average().getAsDouble())) .values() .forEach(System.out::println); }


2

Actually I did not reviewed your code as such (as warned by @Emma). But for an "architectural" perspective I see some improvements. However some are more personal guts than real good practices. The first reflexion is about your the motivation between those two microservices. Why did you split reporting from anything else. Why did you group Account, ...


5

I can agree with all of in two previous answers, however I have to address the issue of implementation of game logic itself. I just so happens that we understand the game logic of the game because we all know the game, but if we look at the code it is difficult to extract the game logic from the code. It is not written in clean, easy to understand way, ...


5

Nice implementation. Few suggestions to make the method groupByMean more compact using Java Streams: Calculate the average: public static double getAverage(int[] arr) { int sum = 0; for (int num : arr) { sum += num; } return ((double) sum) / arr.length; } To: public static double getAverage(int[] arr) { return Arrays.stream(nums).average()....


Top 50 recent answers are included