Tag Info

1

Soo ... I'm not quite sure how to formulate this without it sounding brash, but... Both of these methods are less than ideal... Let's follow the behavioural flow to see what issues this code has: The webservice portion you provide only understands PUT requests. Conventionally, PUT requests are used to create objects, not to update them. Instead one would ...

1

I would suggest creating a method for text so you don't have System.out.println every time, but instead have something like text("Text here..."); or something similar. This is done with the following: public static void text(String text) { System.out.println(text); } And to call the method, you type text("Text here");, like mentioned above. This may ...

0

Your code looks great! Here, maybe another option that we might exercise would be to possibly do the entire task with an expression, maybe something similar to these: ^(.+?\/).+\/(.+)\$ (.+?\/).+\/(.+) Our first capturing group is non-greedy, collects our desired path1 for both inputs, followed by a greedy .+ that'd continue upto the last slash, and our ...

2

This is a great project to do. I did a similar project in Swing; except using Clojure's Seesaw wrapper library. You can add many interesting add-on panels to it. My favorite panels that I came up with were a panel that lets the user decide how they want things colored, and one that allowed multiple images to be saved to disk in parallel. It was a great ...

1

Your code actually doesn't work, because you use replaceAll. This means that your pattern will allow you to match ../../ and replace it with , resulting in a lost double-back. You can fix this two ways: You could change the replaceAll to replaceFirst You could change the pattern to exclude ../../ ([^/.]+/\\.\\./? works) You can then simplify your loop ...

0

You are computing a rank 4 times here. Why not store the results in a variable and compute only 2 times? public int compare(File f1, File f2) { if (computeFileRank(f1) > computeFileRank(f2)) return -1; if (computeFileRank(f1) < computeFileRank(f2)) return 1; return 0; } }; public int compare(...

2

Your second code, while might be slightly more efficient than the first due to the avoidance of the second try ... catch inside the first, has two bugs: First, if str contains a value greater than $2^{63}-1$ or less than $-2^{63}$, while it is an integer value (d % 1 == 0 will be true), it cannot be expressed as a long, so d.longValue() will discard the ...

0

You've used a readable idiomatic Java naming convention. There is no point reassign p to player and e to enemy, use the full name on entry, make them final. Static methods are a code smell in OO programming, they will result is tightly coupled code. Follow the tell don't ask idiom, tell the player to attack the enemy and tell the enemy to attack the player ...

2

Have you considered the possibility of ScoreList<T> implementing List<T> directly? This can be easily achieved by extending AbstractList. However, you would have to be careful not to violate the contract of List when implementing your desired functionality: For example, your add(T) method adds an element in a way different from any of the add ...

0

Looks good to me on a cursory glance. It's pretty much the exact same code as in java.util.concurrent.locks.Condition JavaDoc. The only definitive problem I see is the catching and logging of InterruptedException. The code acts towards the caller as if the operation succeeded while nothing was done. Not providing a way to set the maximum wait time will be ...

2

Your class is non-testable mainly because : it reads datas from disk/network it doesn't use any injection Firstly, please note that Spring has some utility to read property file and fills java beans automaticly. You should check this if you don't want to reinvent the wheel. ;) Also, Properties is a fairly old school class in Java and isn't used that much ...

4

You want a list of elements, sorted to descending order by highest perceived value where least valued element is removed if an add operation causes the list size to exceed it's limit. private final boolean highestIsBest; This adds unnecessary responsibilities to the score list. It shouldn't care what the perceived value of a score is. Just pass a ...

3

Deque If you need both Stack and Queue operability, consider using a Deque. Summary of Deque methods - First Element (Head) Last Element (Tail) - Throws exception Special value Throws exception Special value - Insert addFirst(e) offerFirst(e) addLast(e) offerLast(e) - Remove ...

2

As an Iterable is a functional interface with a method just returning the iterator, you could also write: Iterable<T> it = () -> stream.iterator();

4

Please use best practices in general when dealing with password hashing. If you have to verify the database for your generated salt to be unique, you're using a bad salt generator. Make sure to focus on using a good generator instead. Your hash function also seems weak. Consider using key stretching. This should not be required: while(isFound){ isFound=...

2

I took your code and rewrote it a bit, so that it satisfies my personal requirements for production-ready code. if (args.length < 2) { System.err.println("usage: Task <arg1> <arg2>"); System.exit(1); } I removed the throw new Exception since passing the wrong number of arguments is not a programming error but a wrong invocation of ...

0

Usually, we throw an exception if some pre-condition or assumption we've made about the code is violated. So unless processResult assumes/requires that the two string arguments will be equal to do its job, I wouldn't throw an exception. When the program is as small as this, it could be okay to change the initial check to: if (args.length != 2 || ...

1

First of all - I do not know the game and just want to point out some code related things. Thanks :) Not a Getter From the JavaBeans naming conventions: By default, properties are determined from get/set access method combinations that follow a prescribed naming convention. [...] public int getMood() { return mood; } The name of the method ...

1

Review You have redundant code. Store position + velocity in a variable. Use well-known variable names when the code is of a mathematical nature. Distance x Velocity v A clamped area [a,b]. It is not clear from your description what should happen if the distance is so far away from the boundaries, that when reflecting the movement you are still out of ...

6

Your algorithm, if I understand it correctly, is to recursively enumerate all permutations and then check every one of them until a solution is found. There is a very simple improvement to that which can skip huge chunks of the search space: try to detect violations of the constraints as early as possible. For example, if the current permutation starts with ...

8

First, Vector shouldn't be used here. It's essentially a synchronized ArrayList, and you don't need any synchronization in this case. Just change it to an ArrayList. This has the potential to be a little faster. Second, you're using a raw ArrayList which isn't typesafe: list.add("Some nonsense"); // Doesn't cause an error and is necessitating your (int) ...

2

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of ranges happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument: private static int sumRangeLengths(Range[...

3

I think RangeCollection would be a better name than Ranges. To my mind, the RangeIterator is doing too much. It's iterating over a collection of ranges and over each range in the collection. To me it would make more sense for the Range class to have its own iterator and the RangeCollection its own iterator

1

You've received some good comments on the detailed coding, so I'll focus on the design. Follow the single responsibility principle in your controller, it should do one thing, dispatch the commands it receives. The SRP will make it unnecessary to change the controller to add new commands. Replace your switch with a command mapping, see the Command Pattern. ...

0

This looks like a fairly common exercise (in futility?) A common scenario that can't be represented here is be a lifeguard who is promoted to a manager but still does a few shifts saving lives. The position should be a property of staff. Checking pH should be a task that has a requirement for managerial position from whoever is assigned to it. It has to be ...

7

Critical error private static String username = ""; private static String password = ""; private static String queryString = ""; You should never keep request-scoped data in a field in a servlet. Servlet is created only once during application startup, and shared among all requests. So if several clients connect to your servlet at the same time, they might ...

5

package de.practice.Presentation That's not a valid package name under the Java convention. Packages are all lowercase with maybe underscores. private static String username = ""; You don't want these static, actually, you don't want them as fields at all. When dealing with requests, you have to consider that one instance of the same class might handle ...

3

@vajk already mentioned most important points, but I will add comments on some things that I see. patronCount is redundant, you can simply use patrons.size(). Both arrays that are Pool members can be made final. phLevel is a double meaning you have to work with floating point. You have to consider which values are used and perhaps better to use integer, ...

2

Some nitpicks on top of the already excellent review by Benjamin: Use the standard library's Objects.requireNonNull over checkNotNull The way you obtain the enum values feels a bit contrived. Consider using values() instead. It's a bit annoying that java's generics are as weak as they are, making it impossible to "just" call X.values() (which is guaranteed ...

4

Is how the current project is built acceptable? Is it coded in the most efficient way possible? That depends on your definition of effiency. I guess you mean in it OO-Terms, i.e. maintainability, encapsulation, decoupling, etc.. In that terms, yes, your program has a good encapsulation and a good overall structure. I do not know the exact purpose of Staff....

1

Design Since duplicates are allowed (no Set required) and you don't need to fetch books by name (no Map required), ArrayList will do just fine. Overview Collection Types Guard public entrypoints Make sure to checks arguments on all publically available code that takes input from calling code. if (novels == null) throw new IllegalArgumentException("...

2

Your code looks good however, I have few suggestions. private String OnlyAlphabets the convention is to use camel case for method names, so it should be private String onlyAlphabets Although this is your sample program however, if such small utilities are required throughout the application then make them static and public. This will allow ease of access ...

5

Your code looks good for very simple text. However, real life is much more complicated. In languages other than English, there are characters that are combined from other characters. For example, the German Umlaut ä can be written either as \u00E4 or as an a, followed by the combining dots above, which is written a\u0308. Both representations look the same, ...

7

char ch; In general, we want to declare as late and at as small a scope as possible. You never use this outside the loop, so it could just be char ch = string.charAt(i); inside the loop. Even better, use the range-based/foreach style: for (char character : string.toCharArray()) { Now you don't have to worry about i or charAt at all. ...

6

You code seems pretty straightforward. However, checking for true rather than false is probably more clear. if (Character.isAlphabetic(ch)) { sb.append(ch); }

2

In JPA you could use JpaSpecificationExecutor<T> in repository, with that you could utilize it's method. TransactionRepository public interface TransactionRepository extends JpaRepository<Transaction, Long>, JpaSpecificationExecutor<Transaction> { } If you look at the source of ...

8

Sorry, but that someone is probably right. At the same time, thanks for being thus brave and asking on how to improve here. It's not so simple to give feedback in such a scenario, since there are so many details that go "wrong". Since you tagged this for beginner, I'll focus on things that will benefit you most as well as things you can easily adapt. You ...

3

Code Beautification is suggestive. This is how I would write your code. There is also a possible performance optimization by only calling super.getItemCount() once. be consistent with parentheses (I'm using a different rule for methods and conditions, but feel free to use your own preference as long as you are consistent) create one variable to store the ...

6

All the other answers are very good and detailed but they fail to touch upon the reasoning behind the fatal flaw in your code: static variables are generally bad and to be avoided. This answer should help you understand some of the basics of static variables so you can understand the other answers better. To explain that we need to touch on some OOP basics ...

21

Let's take this from the top down: /** * Title: GameFunctions * Author: Dev * Date: 6/1/2019 * Purpose: This class contains - * -all the functions for the game * */ public class GameFunctions This comment doesn't tell me anything new: Title is already mentioned in the class declaration that follows Author and Date are already kept in version control ...

2

Let's consider the results of numbers around 10 as an example : 7 % 10 = 7 8 % 10 = 8 9 % 10 = 9 10 % 10 = 0 11 % 10 = 1 12 % 10 = 2 13 % 10 = 3 We know that 8 to 12 should be included. For 10 to 12 it's easy, we just need to check if their modulo is smaller than 2. public boolean nearTen(int num) { int modulo = num % 10; return modulo <= 2; }...

3

I do not know if it is a good tip ( I am a beginner too), but I think it would be a better practice to initialize other classes at the very top using constructor. You instantiate scanner twice, together with removing all static modifiers, you could instantiate scanner and random automatically when you instantiate GameFunctions class in main class.

5

You should read about congruent integers and modular arithmic. The distance of a number to a base is the minimum distance of its congruent value and its inverted congruent value. public boolean nearBase(int num, int base, int deviation) { // ..TODO check guards, normalize input or throw exceptions // - base and deviation are expected ...

3

In addition to Himanshu's answer. Move the instantiation of Random to the class level. private static void setRandomNumber() { Random random = new Random(); int low = 0; int high = 100; randomNumber = random.nextInt(high - low) + low; } You are creating new instance of Random every time you call the method. ...

8

I have some observations. All your methods & fields are static: This is not required. You can remove the static modifier. Getter Methods: private static void getAttempts() { System.out.println("Attempts remaining: " + attempts); } A getter method is expected to return some value and not print some value. If you want to print attempts then rename ...

3

You implemented convertByteArrayToHexString really inefficiently because you allocate a new string for each byte that is converted. Also, there's no need for a StringBuilder since a character array suffices. The idiomatic code for converting a byte array to a hex string is: public static String toHex(byte[] bytes) { char[] chars = new char[2 * bytes....

2

The solution of @AJNeufeld is elegant and solves your immediate problem. However, I want to make a case for doing things the verbose way: check for each bad condition individually, log an error if the value is null, and continue onward otherwise. This has two advantages. First, it allows custom error messages depending on which value is null. If a missing ...

6

The problem with catching NullPointerException is “which one did you catch?” A null can be returned from getGrandParent(), and using that return value without checking will cause the exception. OR a bug in getGrandParent() might cause an exception while trying to find the parent’s parent, and you are obscuring the bug by assuming the NullPointerException ...

-2