# Tag Info

-3

import java.util.*; public class Adress1 { public static void main(String[] args) { // TODO Auto-generated method stub Adress1 ad=new Adress1(); Scanner sc = new Scanner(System.in); int menu; String choice; System.out.println(" 0. Exit. "); System.out.println(" 1. Add contact. "); ...

1

E.g. in the getSeveralProperties() method, I have used switch inside switch, and my code became duplicated, not DRY. You can fix this by creating a Property interface with implementations for even, sunny, etc. Then, rather than using nested switches, you could simply say for (long i = 1; i < top; i++) { if (Numbers.holdsAll(number, properties)) { ...

1

Here are a few comments on your code: Business objects If the business objects' model is given, there is nothing to do here. Otherwise, just the way you have described it: Price - Price of book (Can have multiple prices for same book... Why is prices a field of Author? It makes way more sense to let each book have a list of its prices. That way, you do not ...

4

Your algorithm is correct, but the performance can be improved to O(n) avoiding the sorting of the n elements array with O(n log n) cost. The final result is equal to max + 1 - min - n where min and max are respectively the minimum and the maximum element in the array, so your method can be rewritten like below : private static int makeArrayConsecutive2(int[]...

5

Indentation You indentation is inconsistent. int count =0; is not indented the same amount as the following line, despite being logically at the same level. Your indentation amount is not consistent. You should use the same increment for each additional level. Assuming your first indentation was 2 spaces (instead of both 1 & 2), your next indentation ...

1

As far as I can understand, you're a beginner lacking some knowledge. But the job is done well. Some considerations: Comments A perfect code should use identifier names to avoid comments; your code have good names and one unneeded comment at the beginning. But, on the other hand, you have good task description for comments. You can break it into comments, so ...

0

Naming: Uppercase names are usually reserved for constants. addOrCreate : Why are you checking orderId twice? computeIfAbsent will check if the orderid is there, there is no performance or any other gain in doing it beforehand. Then you are getting it for the third time, however your function is not atomic. So it is possible that it is not going to be there ...

1

I want to add to the other answers and provide a different more drastic suggestion and a more generic and (I think) clean approach. This is the perfect example where a nice class hierarchy shines. This is also the perfect example where to use visitors. I think in this case everything should be an expression. First, the problem can be broken down in separate ...

4

A couple of comments on your answer: Code style If you've already stored the pattern.length(), why keep calling the method? Just be consistent and use that variable. Same goes for str.length(). I'd just store it and use it. I would store str.length()-(pattern.length()-1 in a variable an name it something like numOfCasesToTest, to make it clearer. Check ...

2

It is possible to achieve a much better performance in a brutal way result with the use of auxiliary sorted k size array obtained copying and ordering the first k elements from your nums array: int length = nums.length; int lastIndex = k - 1; int[] arr = Arrays.copyOf(nums, k); Arrays.sort(arr); Once you initialized the arr array for every remaining ...

3

You need to be observant of the bounds in the problem description, in particular they allow k=n which means your worst case time complexity is not constant k times O(n), it's actually O(kn), and when k is approximately n, then it turns into O(n^2). I solved a similar problem a while ago and did some interesting benchmarking. I'm not sure why you don't want ...

1

Naming MathResult; I was wondering why the prefix Math in MathResult is important. Are there other types of results? If not, we can keep it simple and only name it Result. Operation; As @Ralf Kleberhoff already points out, would Expression a better fit. The grouping of operations and operands is an expression, where an operation is a function such as ...

1

(I've read the book, solved the problems and passed the interview) The purpose of this exercise is to use the properties of lists to your advantage, and show your understanding of them; as well as teaching you to ask the right questions: How big can the numbers be? Will they overflow an int/long? Can they be negative? Will both of the lists fit in memory? Is ...

1

Input doesn't need to be a separate source file when you can use an anonymous inner class that implements KeyListener. When you do this, it becomes easier to put the variable paused where it belongs (i.e. not in the class handling the input). See my edits below for how to do it. Similar thing with Window - it doesn't need to be a separate file, and we don't ...

1

Welcome to the site! This looks good so far, here's a couple of smaller tips: Board.java The cellsize doesn't change, so why not make it a constant like this: public static final int CELLSIZE = 4; This also makes the getter obsolete, as you can access that field using Board.CELLSIZE getNewState() uses a boolean state. Renaming that to something like ...

2

Your indentation has gone wrong here, and it's confusing: if (sl.equalsIgnoreCase("M")) { reliefDays = "SUN MON"; if (rdLength == 3) { reliefDays += " TUE"; } if (rdLength == 4) { reliefDays += " WED"; } ...

3

Naming What you call "operation", in most cases I'd rename to be an "expression". Documentation You create a quite complex set of classes and interfaces, and the only hint about their usage is in the class and method names. You should add Javadoc comments at least to public classes, interfaces and methods. Interface OperationParser public ...

2

Basics First of all, you've got the basics right, being naming conventions, indentation, and a rather sane OOP structure. But you ask for review, so here I go. Reinventing the wheel You probably know the Timer class, doing exactly what you implemented. So I guess, your program is meant as an exercise. Use proper names Your class is named Ideone, and that's ...

-1

What I couldn't deduce from the problem description was the definition of "going right" in the last column (usually it means "go the first cell of the next row") but here it'd might just as well be "not allowed". Same question for "going down" by symmetry. I next tried to deduce the definition from the code, but ...

0

Thanks @RobAu to provide solutions, but there is a requirement that the head words should be sorted as well. We need a custom class for TreeMap class PrimeKey implements Comparable<PrimeKey> { BigInteger primeHash; String keyword; PrimeKey(BigInteger primeHash, String keyword) { this.primeHash = primeHash; this.keyword = ...

1

Recursion is good only if you know its depth If you have a tree with maximum depth of 100 or so - recursion is normal to traverse it. But here you can get a million steps of recursion for a stupid example of gcd(1,1000000). Change it to the loop. GCD can be calculated faster if you change subtraction with modulo Because (a-b)%b === (a%b)%b, gcd persists. ...

1

Your space complexity looks like it is proportional to n (O(n)? I'm unsure of the notation). That doesn't seem bad to me, and I don't think it can be improved (but don't take my word for it, I'm not at all an expert). However, let me try cleaning up your code a bit. It's well-formatted, but there are some minor things that can be improved. args is the ...

0

I won't repeat what XTros and Martin Frank have said... This is code written in Java, but it's not very Object Oriented as Martin pointed out. To reduce the confusion here, I'm going to use the term StudyGroup for your Classes, so that when I talk of classes in the rest of this post, it's clear I'll mean Java classes. If I were implementing this, for a start ...

2

You should rewrite your reverseString to use the built-in StringBuilder.reverse(). Your class structure is a little bit mixed up. The Application and Main classes currently have some of the same obligations. Rephrase this to, perhaps, a Main class that only cares about display and does your program loop a StringInfo class that does no integrated display in ...

0

Major issue: Java Basics OOP where are your objects, where are your methods? yes, we do have a class Students but what else? All of your logic happens within Playground, which can be a suitable class for an excersize, but right now it's totally bloated up! here are some ideas for objects StudentGenerator - has a method generate(int amount)... Grade - yes, ...

3

This is not a full code review, but some things to start: Double check your indentation The properties in Student should be marked as private. It's customary in Java to always have these fields marked as private, then provide getters/setters to them. This could look like: class Student { private int number; private double grade; public int ...

1

Your first code snippet seems to use prepareFileData to inject string-encoded rows into a String-buffer. Every CHUNK_SIZE rows, the buffer is appended to a file and the buffer is then reset. That makes some sense, but: If the number of rows is not a multiple of CHUNK_SIZE, some rows will be not be written and hence lost. Mixing abstraction levels (here: ...

1

The sample code is too small, and lacks enough context, to allow a detailed analysis. Initial points though are: Your method name is poorly chosen. It doesn't create anything, it looks something up. Your if statements immediately return, so there's no need for "else". In the IDE I use (eclipse) I'd get warnings about that! The suggestion to go ...

0

In the latter java versions its even much easier. I would recommend the following:- private static Item createItemType(final Integer type) { if (type != null) return switch (type) { case 0 -> Item.PLACE; case 7 -> Item.ADDRESS; default -> return null; } return null; }

11

switch I would find a switch cleaner. private static Item createItemType(final Integer type) { if (type != null) { switch (type) { case 0: return Item.PLACE; case 7: return Item.ADDRESS; } } return null; } If you really want to reduce the number of lines, you can combine ...

7

In this case, i would use the inline if. This would take less line and would be cleaner overall. private Item createItemType(final Integer type) { return type == null ? null : type == 0 ? Item.PLACE : type == 7 ? Item.ADDRESS : null; }

2

I have a lot experience with Java, but still, code is a very subjective thing, so on many things there is no single universal truth. Rename Application to AgeCalculator Application implements Runnable Encapsulate(make private) methods printInstructions and loopProgram in the Application and call only run from main. You are forcing the user of Application to ...

0

The term "list array" is inapproriate (it's generally understood to mean an array of lists). What you're doing is rotating left (by a distance of 1) d times, while it would be more elegant (and efficient) to rotate left (by a distance of d) just 1 time. Also, in the process you're changing your input (parameter a). That's not always evil, but it is ...

10

major issue: misunderstanding of exercise you should find the amount k of friends - not the friends within an distanz k major issue: bug for(Pair pair : list){ double d = distance(pair); map.put(d, pair); } if two "pair"s have the same distance you'll lose one result major issue: naming sounds obvious but you should create a Position not a ...

1

You should not need to do this yourself. Consider d %= a.size(); if (d == 0) { return a; } List<Integer> results = new ArrayList<>(); results.addAll(a.subList(d, a.size())); results.addAll(a.subList(0, d)); return results; If d is initially 0, it will rotate in constant time. This is the same as your function. If d becomes 0 (when d is a ...

0

Is it a list or an array you are to work with? They are not exactly equivalent. If the data is an array, you can perhaps use System.arrayCopy() in your solution. Do you have to rotate in place or can the result be a new array (or list)? If you are rotating more than one place, you should be able to do so in one go, rather than multiple single-place rotations,...

0

An important omission Consider this snippet: if (currHash == (hashPattern * primePower[i] % MOD)) { return true; } It returns true when the hashes match. But hashes are not unique, they cannot be, there are fewer of them than there are strings. So there should be a proper string comparison here, to distinguish false positives from true positives. False ...

1

Some minor changes could be applied to the code, for example the following lines : ArrayDeque<Rectangle> rectangles = inputProvider.getRectangles(); public ArrayDeque<Rectangle> getRectangles() { ... } private final HashSet<Point> disjunctiveCorners = new HashSet<>(); public boolean check(ArrayDeque<Rectangle> rectangles) { ... }...

0

I noticed in your Byte class the following code: public static byte[] merge(byte[]... bytes) { final ByteArrayOutputStream stream = new ByteArrayOutputStream(); try { for (byte[] b : bytes) { stream.write(b); } } catch (IOException e) { System.err.println(e.getMessage()); } return stream.toByteArray(); }...

1

Interestingly, the thing that most facilitates a simple row-wise iterator is to arrange the storage in the same format that we'll iterate. Instead of having an array of references to per-row arrays, representing a matrix by a single linear array is a good choice: faster to create, and with better memory locality: public class Matrix { private int[] data; ...

0

First of all, kudos for figuring out that $\displaystyle \prod _{i=L+1}^{R} (A_{L})^{ A_{i}} = A_L^{\sum_{i = L+1}^R A_i}$. But - you've stopped too early. The next step is to realize that $\displaystyle \sum_{i = L+1}^R A_i = \sum_{i = 0}^R A_i - \sum_{i = 0}^L A_i$ which hints that you need to deal with partial sums of $A_i$. This way you don't have ...

0

if you have two entities that have not so much in common i suggest to avoid strong coupling! any method that refers to the other entity looks wrong to me: House.upgrade(Player player) Player.upgrade(House house) even though you don't use the Entities as parameter in your method (it's not obvious in your code where it comes from) you have this strong coupling....

2

Readability first Indent the code properly. Put every statement in its own line. Name identifiers properly (well, this is mostly done). Separate the algorithm from input/output operations I/O operations are slow. Sometimes it's ok to mix them, but if you want to benchmark your algorithm - move all I/O out of it. The best way would be to create separate ...

1

Your Java 8 example is less efficient because it iterates over the string's code points twice (once for the countUpperCase method and a second time for countLowerCase). In your first example, you can optimize the code by adding an else clause to the two if statements. Because, if a character is uppercase, you know that if cannot be lowercase, so there's no ...

2

What you wrote is really good for someone that just started programming :) What you can do now is look at your code from a higher level Initial comments // this is not a todo list or "only" a todo list public class ToDoList { // initialization - nothing much to see right now except naming // userInput should be a scanner or sc // ...

3

The code is correct from a realistic point of view but implements a stricter criterion than actually stated. Consider e.g.: 2147483647 + 2147483647 + (-2147483648) + (-2147483648) Here, the result could be a "solid" -2 but CanAdd rejects it nevertheless. As such, the code does not satisfy its requirements (but, in this case, I'd almost certainly ...

2

Domain-Driven (Game) Design Naming and terminology of a game (and other applications) follows a convention (depending on its genre). Objects are typically expressed by nouns (N) like: Health and Player, Item, Inventory, House, Room, Location. Methods are typically expressed by verbs (V) like: refill, regenerate, store, add, move, enter, etc. Depending on the ...

Top 50 recent answers are included