New answers tagged

1

Unit Tests I downloaded the code from the question and then stubbed the classes that weren't present (Token and Player). When I ran the unit tests, they hung. If you're treating the whole code base as a unit (which you appear to be doing), then they should all be included in the question. However, it seems like a red flag that the test hung, rather than ...


2

Format So first up, as has been said in the comments, code formatting is important. Consistent formatting makes your code a lot easier to read. If it really looks the same in your editor as in the question, then you should try reformatting it in a good Java editor: public int issue_book(Issue_Use_Bean obj_Issue_Use_Bean) { Connection connection = ...


1

@BeforeEach void init(){ ModelMapper modelMapper = new ModelMapper(); passwordEncoder = new BCryptPasswordEncoder(); You're using actual implementation classes from your unit test. This has implications in that you're also effectively testing these classes, so you have potential side effects from changes when you upgrade the library, performance / ...


2

Your code has a bug. I can reproduce it fairly consistently by changing your run method to: @Override public void run() { try { for(int i=0;i<100;i++) { uniSexBathroom.occupy(person); } } catch (InterruptedException e) { e.printStackTrace(); } } Notice, that I've increased the number of times that each ...


3

Graph is a strange name for something designed to manipulate friendships. As indicated by @greybeard in the comments, it doesn't really represent anything at the moment. print I hate seeing methods like printFriends on classes. They very rarely belong. If you want to have the class provide a method/toString implementation that can be used by it's ...


3

Attention to Detail What seem like trivial mistakes can be interpreted as a lack of attention to detail. So whilst your interface has a simple typo claculateUnitForSingleDay vs calculateUnitForSingleDay, it is noticeable. You don't want this slipping into production code, particularly not on an interface or an API object because you can find yourself ...


3

Preliminaries: there's strategy, and there's tactics. A somewhat common procedure to tackle performance problems is to look at "inner" loops first - not entirely wrong, but the golden rule is measure. (And, when turning to others for support, provide measurement results and a test data generator or test data.) Some statements regarding edit distance: • ...


3

As has been pointed out by @Joop Eggen, you should decide if Node is part of the interface or not. As it stands, you can directly access the value of head and tail via the accessors (getHead,getTail), however you can't get at any of the other values in the other values you've stored in the LinkedList unless you happen to have kept track of the Node that you ...


3

Indeed the cases for delete can be reduced. public void delete(Node node) { Node prior = null; Node current = head; while (current != node && current != null) { prior = current; current = current.next; } if (current != null) { if (prior == null) { head = current.next; } else { prior.next =...


1

As said by @TorbenPutkonen, if you have the ownership on your dao classes then you can parameterize them. abstract class Repository<X> { private LiveData<List<X>> allEntities; private Dao<X> dao; void insert(X entity); } abstract class ListingFragmentViewModel<X> { private Repository<X> repository; public ...


2

In isAnagram one should start local variables with a small letter (just a convention). Do not use if-else to return a boolean. public static boolean isAnagram(String str1, String str2) { String s1 = str1.replaceAll("\\s", ""); String s2 = str2.replaceAll("\\s", ""); boolean status = true; if (s1.length() != s2.length()) { status = ...


1

You seem to have started down a singleton approach, then changed your mind / not completed it. private static DbConnectionPoolManager instance; public static DbConnectionPoolManager get(DbPoolconfig dbPoolconfig) { if (instance == null) { synchronized (DbConnectionPoolManager.class) { if (instance == null) { return ...


0

You have used sorting which has a time complexity of O(nlogn) You can use HashMap ,by which time complexity of program would be reduced to O(n)..Below is the code static boolean areAnagram(String str1, String str2) { HashMap<Character, Integer> hmap1 = new HashMap<Character, Integer>(); HashMap<...


2

Interface There's a name conflict with java.util.map I'd try to avoid using names that are the same as existing library classes. Does print really belong on this interface? If you really wanted to have it as part of the implementation, it could be there without needing to be on the interface. However, it would be better to provide some kind of iterator (...


3

I'm going to assume this question is in the context of quality interview code, not quality production code. Websites like GeeksForGeeks (https://practice.geeksforgeeks.org/problems/jumping-caterpillars/0) don't grade you based on production code quality. Your failure to correctly spell the most important word in the problem domain (Caterpillar) would put ...


1

Based only on the code, I have suggestions: For the variable, I suggest that you mark them private to prevent exposure; especially the collections. private Queue<Integer> myQueue; private int size; I think the comment on the constructor is useless. You can remove the type java.util.LinkedList in the diamond operation, in the implementation ...


0

You could build your final string using ternary operators and avoid all the ifs. I think this is much more readable: String firstChar = str.charAt(0) == 'a' ? "a" : ""; String secondChar = str.charAt(1) == 'b' ? "b" : ""; String remainingChars = str.substring(2, str.length()); String finalString = firstChar + secondChar + remainingChars;


1

A few subjective points... Isolation Going forward, you're going to want to isolate the UI from the algorithm as much as possible. So, using methods to break up the algorithm as suggested by @Doi9t is definitely worth considering. I think it's also worth considering making the output stream a class field for the moment. This is going to be closer to the ...


-2

you can try this as well. public static String deFront(String str) { if (str.length() >= 2) { if (str.charAt(0) == 'a' && str.charAt(1) == 'b') { return str; } else if (str.charAt(0) == 'a') { return str.substring(0, 1) + str.substring(2); } else if (str.charAt(1) == 'b') { return ...


6

Document your code. In the code. • what use is MovingAverage? • is size fixed for the lifetime of any given MovingAverage instance?  I don't know even scrutinising the code: size is neither final nor private • what does next(int value) return? accumulate ints into longs - even the sum of two ints can overflow. program against interfaces - and give ...


1

Parameter for sorting the data (ASC, DESC) If the task actually used 'ASC', 'DESC', then I'd say that there's a good chance they were expecting you to use an enum to make it easy to select the sort type. If not, then personally I'd have preferred a boolean for the ascending parameter. Using magic numbers like '1' to indicate sorting direction is going to ...


0

I have one advice, but since the code is not compiling, it's a bit hard to make a proper review. Instead of using an java.util.LinkedList, I suggest that you use the java.util.concurrent.ExecutorService to handle the threads and use it to check the end of the threads. ExecutorService taskExecutor = Executors.newFixedThreadPool(4); ...


1

A potential issue is related to the static Scanner object: private static Scanner scanner = new Scanner(System.in); This scenario can bring potential problems due to the closing of the Scanner resource left to the developer. From Java 8 it is possible to use the try-with-resources statement to ensure closing of resources (in this case the Scanner object to ...


0

No, your code is not good enough. It lacks the very basic API for an encryption and decryption module, which is to provide the secret and public key via a public method. This means that you cannot decrypt a message that was encrypted in a previous run of the program. You should follow the Java Naming Conventions: Method names, field names, local variables ...


3

I have some suggestion for you. 1) Constant name should always be uppercase. private static final Scanner SCANNER = new Scanner(System.in); 2) When creating arrays, I suggest that you use the java style instead of the c style, since it's the more used and less error-prone, in my opinion. Before public static void main(String args[]) { //[...] } ...


1

Care about final Your parameters are never modified, so it is good practice to mark them final. Probably the method too, unless you expect it can be overridden. public final int shortestDistance(final String[] words, final String word1, final String word2) Compare by indices Instead of your for loop, you can use the indices of the elements. public final ...


1

Assuming the requirement was to maintain initial map as immutable object -- just a few concerns re those new statements in sorting. They violate SRP, at least. Either accept already created (mutable) to populate with results, or (I prefer) accept a factory to get one. Same apples to collection step in filtering. If, however, it is given that input map can ...


2

First off a typical bad habit many beginners get into is, putting everything in main. If the code is only doing one thing and can be self contained a function will do. Otherwise, use a class. In this case I would suggest a class with static fields and functions. Try to avoid using magic numbers, strings, etc.. Use final variables so that they can be ...


1

I found a little inconsistency in the following part of the code: // Minuten berechnen int minutes = seconds / 60; seconds = seconds - minutes * 60; System.out.println("Minuten: " + minutes); In all the other paragraphs you use named constants. You should follow that pattern here as well. MINUTE_IN_SECONDS is a perfect name for the 60. The following two ...


3

I have some suggestion for you. 1) The naming convention of the variables HOUR_IN_SECONDS, DAY_IN_SECONDS and YEAR_IN_SECONDS is not correct, since they are not constant; missing the static keyword. I suggest that you extract them in the class level. public static final int HOUR_IN_SECONDS = 60 * 60; public static final int DAY_IN_SECONDS = HOUR_IN_SECONDS ...


0

int start = 0 int end = 0 int total = 0; int target = 8; while(end < arr.length){ total += arr[end]; // 1 3 1 4 = 9 /* Adding this part is enough as well */ if(total == target){ return true; } /* Adding this part is enough as well */ while(total>=target){ total-=arr[start]; // 8 start++; if(total == target){...


6

I am %100 sure that this is the correct answer, I'm not so sure... I dumped your code into an online compiler*. I made very minor adjustments, moving target to a parameter and adding semicolons which aren't a big deal since you probably didn't have a compiler when writing it. I did not modify your code beyond that and treated it like a black-box. import ...


2

I don't know Java, but I still found your challenge interesting! So I translated your code to Python and tried to beat it: def your_search(seq, target): start, end, total = 0,0,0 while (end < len(seq)): total += seq[end] while (total>=target): total-=seq[start] start+=1 if (total == target):...


0

All suggestions posted before my answer are valid, I'm writing some personal thoughts about your code: public static void extractBirthdayFromRodneCislo(String rodneCislo) { //omitted System.out.println(year + " " + month + " " + day); } Instead of printing values inside this method you can return the String object as a result and after print it like ...


3

Others have improved the parsing, but the code surrounding it is lacking. You asked for help for parsing the date but if the code should be made more "elegant", then instead of defining a single purpose static method for accessing the birth date, you should define a class to represent the Rodne Cislo number. Provide a constructor that performs validation and ...


1

As has been said in the comments, there are bits missing here for a full answer.... however some general feedback. I'm assuming that the job method will be running on multiple threads. samples How important is it that the number of samples collected is exactly reqsamples? If it's important then you need to protect samples. As it stands, it's possible (...


4

In addition to Doi9t's answer, which gives good advice, I'll add that your function name, extractBirthdayFromRodneCislo, is wrong, as it merely prints the birthday instead extracting it. The easy fix is to rename the function extractBirthdayFromRodneCislo, which may fit you case, but doesn't allow you to do anything more fancy than that. The better way to ...


0

There is already a good answer from @Doi9t. I was too slow to publish mine but can still be useful. "My issue is that each color tee has its own "final" data for course rating and course slope, and this makes the calculation differ". Okay, do you know the the strategy pattern ? In computer programming, the strategy pattern is a behavioral software design ...


1

I have some suggestions for your code. 1) I suggest that you extract the different parts of the date in methods; it will make the code more readable and will be easier to refactor. public static void extractBirthdayFromRodneCislo(String rodneCislo) { rodneCislo = rodneCislo.replace("/", "="); int year = getYear(rodneCislo); int month = getMonth(...


0

Some more general things to consider. Your class is called FileAnalyzer, but it advertises public properties like getPrimeNumbersCount, which I'd consider to be part of the analysis result (rather than the analyser itself). Do they really belong there? There's a similar issue around the way that you're storing the file to be analysed in a field of the ...


0

The UpperCamelCase format is used to name types. You should rename your classes (and constructors) to EmailAppender and EmailSender. The ExecutorService has a shutdown method that you can use to reject new tasks and terminate the current ones. You may shutdown your pool form the close method. And there is a nice code sample in the official documentation : ...


2

Here is an attempt to leverage Java's BigInteger class to implement the RSA algorithm, as well as md5 and sha512 hashing functions to generate keys for what I hope to be strong cryptography. My prayer is that some of my ideas are novel and not just wrong. If you start at cryptography, you should not start by making up your own schemes and hoping that they ...


4

Like bhathiya-perera and yourself suggested this can be better implemented using a HashMap and simply count the characters. However in my opinion your attempt isn't that bad. The problem is you are using the wrong methods and data structures and you are coping/creating strings too much. First you don't need to create charArray which is an unnecessary copy ...


5

I have some suggestions for your code. 1) The input variable is unused. 2) Since the tees variables are hard-coded and have two values, I suggest that you use an Enum to hold the values. It will make the code easier to read and remove the variables from the method. Tees.java public enum Tees { BLACK(146, 74.6F), BLUE(139, 72.4F), WHITE(129, ...


1

Single Responsibility My rule of thumb is that no extra methods or data should be added to an enum. If input has to be converted to an enum (or enum to output), a separate Function is created to handle the transformation. This way the enum does not get piggybacked with any additional responsibilities. The enum should be used for computation only. Once you ...


3

You are implementing an inner join as a nested loop join. There are several other ways to implement joins, such as hash-based or sort-merge sort. Which one is better depends on the length of the lists. I will sketch a hash join as that usually works most of the time. void applyFavoriteChangesToMerchantStore(){ List<Merchant> ...


4

All enumerations have a default valueOf(String) method that throw an IllegalArgumentException so your first idea is coherent. I would say that it depends (..). It depends of your use cases: If he conversion would be called form an unsafe (user/api) input, then there are chances that you receive an unexpected symbol. And using Optional will force the ...


5

Let's see what is the Big O of your code. In new versions of Java substring creates a new String and performs in \$O(n)\$ (In older versions this is \$O(1)\$) uniqueRepeats.contains(character) results in \$O(m)\$ where \$m\$ is count of unique characters. restOfString.indexOf(character) != -1 - this is again \$O(n)\$ uniqueRepeats += character Depending on ...


0

Thank you, everyone, who participated in this! New code is on my GitHub. After rewriting code by your advises project looks like this: Project Structure: ST - SingleThread, MT - MultiThread, just for testing and showing the result to you, on Git is uploaded clean version. Main Class: package ee.raintree.test.numbers; import java.io.File; import java.io....


2

First of all, you should read a bit on how to start a Swing application. You must take care of the Event Dispatcher Thread when starting your application. And, because of the way AWT manage threads, you don't need this infinite loop that will just take resources for nothing. The main thread will stay active until your frame is closed[1]. View For the GUI, ...


Top 50 recent answers are included