New answers tagged

4

int day = 4; // Lets number the days of week from 1:7 && Sunday is the first day;;; note : 1 jan 1901 was Tue day The comment gets lost on the right edge of this narrow screen. The position of the comment implies that it is specific to the variable day, but that's not true of the first sentence. For working with % it would be less confusing to use ...


0

Fairly simple code. On thing that caught my eye was the isLeapYear method. I think the logic would be easier to understand by keeping it in one line: if((year % 4 == 0 && year % 100 != 0) || year % 400 == 0){ return true; }


0

This is bound to be very opinion-based (like many reviews are). However, if your business class basically represents a collection of objects, implementing Iterable is the right thing to do. I'd even go so far to say, that not implementing Iterable is a sign of neglect. Extending the class from a given collection type instead is not a good thing, as it ...


0

IIUC, you want the class People to have all the functionality of Set<Person> with some additional custom methods. In this case, one option is to extend a concrete Set<Person> implementation like HashSet<Person>: class People extends HashSet<Person> { // All other methods are inherited from HashSet<Person> public Set<...


3

I'll start this post off with a notice that my knowledge of the Java language isn't very high. However, I have a lot of experience with development in languages like C# and JavaScript so there are a few things I can help with. Most of my code below will be pseudo-code, but I did research more proper syntax and import statements to clean it up a bit. Based ...


4

This looks good. I assume this results in correct answer. What you can use instead of converting to String and back to int is to use divideAndRemainder method with 10 since we need to treat this as a base 10 number. This method is available in BigInteger for situations like this. We can also directly use BigInteger constants such as TWO and TEN. ...


1

Yes, you can get rid of the if. I'd leverage a Function mapToOutput, with value either x -> x (no change) or x -> String.valueOf(x) + System.getProperty("line.separator") (your other logic). Keep in mind that it may not actually be desirable to do this, though - it adds complexity, while only saving you a little repetition. I've done it below, and ...


1

Opening this in an IDE already shows a bunch of problems to fix. input and output can be local variables, in fact this is all rather procedural, so it makes sense that the containing class is just to contain the main method anyway. The empty constructor is pointless. The args parameter is written weirdly, it should be String[] args. throws Exception doesn't ...


3

Please note I am not an expert in Java, I do know something about object oriented programming. Java is an object oriented programming language. To write good object oriented programs it's best to keep 5 programming principles in mind. Together these 5 principles are called SOLID programming. The 5 programming principles are The Single Responsibility ...


0

Don't store method flow flags as state in classes. This is a breach in object-oriented design. class Node { public boolean visited; // .. other } Search methods like dfs should use a map of some sort to store which nodes are visited. By storing this flag incorrectly as state, you'll get in trouble when multiple threads search the graph ...


0

Use a CyclicBarrier with only the neural-net thread waiting, and decrement it after adding an item to the queue (which should probably be a ConcurrentLinkedQueue). Then it will automatically trigger after every 32 entries, at which point it should pop exactly 32 entries from the head of the queue (an operation that won't block if it's the queue's only ...


1

Well, technically you should really post complete code and not have missing definitions. Especially since Data and OtherData could instead be generic type arguments perhaps, or interfaces, so that it's actually clear what's happening. I'm just gonna imagine they're basically both Objects. Also Processor is undefined. If it was really not important just ...


2

From java 8 it is available in the String class the method join that achieves the same result of your method stringConcat so instead of: String k = stringConcat(achat.getCodSociete(), achat.getCodService(), achat.getNumCompte(), achat.getCodCommande()); You can use String.join with "" delimiter: String k = String.join("", achat.getCodSociete(), achat....


1

Review see if I use the global variables, nested loops and two functions with parameters correctly loopCount is a magic variable, its meaning beats me. Is it some kind of debugging assistant? The indentations seem random picks between 4 and 8 spaces. Stick to 4. The div method is public, the mul private. Why introduce this inconsistency? Give full names ...


4

You have a lot of repetition in your code. The only thing different with the 2 lines of code repeated over and over is the argument. You could put all of these arguments into a list and iterate over them, running those 2 lines of code for each: List<String> states = Arrays.asList(isRQMRunning_3, startRQM_4, pendingChangesStatus, ...


0

You know there can only be 101 elements in the result. So allocate a boolean array of 101 elements. Then just read numbers one by one and tick the index in the boolean array, that corresponds to the number, to true. In the end, print the indexes that contain true. \$O(n)\$


4

General Classes should be final unless you intend and have designed them to be extended. Marking local variables as final will clue the reader in that they don't change once assigned, which improves readability. Use full, descriptive variable names. The extra characters don't cost you anything, and they make the code more clear and easier to read. The ...


2

I was curious about why you kept the initial size inside each of the arrays until I finaly noticed the arrays[i][0] in the check if that array still has elements to proces during the algorithm. If we replace that with arrays[i].length-1 we no longer need to store those. Another major issue, like Nathan pointed out is how many String objects you're creating ...


2

Are you getting the correct output with the recommended complexity? It's been a long time since I've written my own sort but start by confirming that before any of my recommendations below. I suspect str.split(" ") is causing problems for you because you're creating lots of strings which you're about to parse and then throw away. How about parsing the ...


3

In addition to what other's have said: Your constant-increment growth scheme causes push operations to be amortized O(n). You should grow by a constant factor, as ArrayList does (1.5x, if I recall correctly). Even better, just don't use a primitive array at all, and use ArrayList instead.


17

Other answers correctly point out that using primitive types and not mixing them with Objects (=int instead of Integer), reducing visibility wherever possible (=adding private modifier to position and list) and preventing popping from empty stack are good practices. There are a few more subtleties which can be improved: Rename INCREMENTSIZE to ...


5

Your class is good (for me I would rename the variable list as arr), but you should consider case where you call pop on an empty stack. In this case your method could throw an exception like the code below: public Integer pop(){ if (position == 0) throw new RuntimeException("Empty Stack"); return list[--position]; } You can check from Java ...


3

The first thing about your code are the two lines inside your class Thing: private Random r = new Random(); private int a = r.nextInt(), b = r.nextInt(), c = r.nextInt(); If Random r is used just to initialize the array and not in other methods inside the class , it is better to use it in the costructor of the class : public Thing() { Random r = new ...


1

The naming seems consistent, that's great. Also overall I don't have much to complain about, for Java it reads pretty well. MerlinSource: I'd be a little bit concerned with the very hardcoded and very specific selectors, especially since something like grid__col--20-80-80 seems like it could very easily change without notice! Maybe some safeguards would ...


10

Your class is small and does what you said it should, that's good. However, there are a few things you could do better. 1. Use primitive types unless the boxed ones are specifically needed: You are using the Integer type for counting the position and storing/returning data. If you actually used the fact, that it could be null, this would be acceptable use. ...


12

This looks good, However I suggest you properly indent the code. Further suggestions: I would change following private final int INCREMENTSIZE = 1024; to private static final int INCREMENTSIZE = 1024; Since you are not changing this in the constructor (to a new value) we might as well make it unique for the whole class. I would change following ...


1

Advice 1: code packaging I suggest you put your Thing related code into a package. That way you may practice industrial level programming: package net.tnm; Note that the above package name is just an example. Usually, is should be reversed domain name of your company. (For example, package com.oracle.xxx where xxx is the project name.) Advice 2: code ...


3

Coor has the comment coordinates ... yeah, that's exactly what the name should be then. But actually, Point seems easier and doesn't have to be abbreviated, or perhaps be more general and say Vector, or Vec2, that seems fairly common for games (despite it being an abbreviation). Not using the AWT class makes sense to me too. The hashCode method is okay, ...


7

The algorithm provided by the OP presumes that the array is important and preserves the original array. In addition, the displayed algorithm seeks an "instantaneous answer", as if the routine may be interrogated at any point to get that answer at the point in time. All that is important here is the output - a single number. Any other information used to ...


-1

This will not improve your efficiency, but it will get rid of the redundancy that you were asking about. public class ProductValidation{ public Violation validate(Product p){ bool local = isLocalBrand(p); if((local && isBlockedLocation(p)) || (!local && isBlockedInterBrand(p))){ ...


5

Better complexity can be achieved using a heap. The input array can be organized into a min-heap in \$O(n)\$ time with negative integers dropped. Then the smallest number can be popped one by one until the target number is found. The complexity of this algorithm is \$O(n + klogn)\$ where \$k\$ is the insertion index of the target number among the sorted ...


8

I then tried putting the array into an arraylist, which reduces big-O since each object is "touched" only once, and I can use .Contains which is more efficient than iteration (not sure if that's true; I just sort of remember reading it somewhere). As was mentioned in the comments, for your purpose, there is no significant difference in performance between ...


2

Perhaps the next piece of code may be overwhelming for a Java beginner, due to the fact that it uses advanced features of the language such as streams, lambdas, and try-with-resource clauses. However, just for showing a different approach using functional programming, your program could be rewritten as follows: public static void main(String[] args) { ...


1

It seems unclear what responsibilities the Cell class is supposed to have. You obtain a Cell from the Board and then query the cell for information about whether it is flipped or not but to flip a cell or get the surrounding cells you have to make a request to Board. To me it would make sense that the operations that are done on a Cell would be done through ...


0

Not sure how using roots help you, but here is my solution to the porblem (a pyramid, n-th row has length of n): EDIT: I assumed that if the number won't be a perfect pyramid (like 24) then print until the number. If you want to print the biggest pyramid possible it simplifies the code a bit. public static String pyr(int n) { //n is the target num ...


1

Both optional collection and a collection of optionals are anti-patterns. There is no scenario where returning Optional<Set<X>> or Set<Optional<X>> is preferable to returning a Set<X> that can be empty. As @AJNeufeld mentions, a guard condition is a good way to start. In your case. If incoming usersIds are null then return an ...


2

Not sure about its efficiency or whether or not it improves your algorithm, but since you want to use streams, this would be an option: public static void main(String[] args) { String sentence = "ala ma kota, kot koduje w Javie kota .kota"; String sanitizedSentence = sentence.replaceAll("[^A-Za-z ]", "").toLowerCase(); String[] words ...


5

Unabbreviated names would be nicer. Immutable properties (row, column) should be made final. Package private directly accessible - especially mutable - fields are not liked. ... have their uses, but not here. {} in generally also are Always used. So: public class Cell { public static final int BOMB = -1; final int row; final col; boolean ...


1

The solution is inefficient. First of all, there is absolutely no need to calculate the set of characters in advance. you can iterate over the sentence and build the map during this iteration. you don't need to know the keys in advance. Furthermore, for each distinct character, you iterate over the entire sentence. there is really no need for this ...


1

You actually have three nested loops there, while two would suffice (the indexOf loops the characters in the word). For each word in the sentence For each character in the word Add the word to the set associated to the character


2

I focus on the operation of storing occurrences of single letters of a sentence in an array of ints and after print the number of occurrences for every single letter, I'm assuming that we are using the english alphabet: int[] occurrences = new int[26]; Now start to examine a sentence and convert it to a char array: String sentence = "Gettysburg, PA"; char[...


4

Welcome to CodeReview! Especially as a beginner programmer, it takes bravery to show your code to the internet, but you've taken the first step. This code has a long way to go, but I think it'll be a good learning experience for you. //declares variables Keep in mind that this isn't 1989-era C, so you don't need to predeclare all of your variables at the ...


2

You're doing things quite the hard way. You don't need to call sqrt, or determine perfect squares at all. Simply track y, and as soon as the row's x == y, make a newline, increment y and set x = 0. The following examples show different results from what you've described: import java.io.StringWriter; import java.lang.Math; class Pyramid { interface ...


2

At first, I think you don't need the value field. Indeed, as shown in createFromInt, you refer to enum values with their position (Option.values()[option]). I suggest the following refactoring. enum Option { ADD_CAR("Add car"), ADD_MOTORCYCLE("Add motorcycle"), PRINT_CARS("Print cars"), PRINT_MOTORCYCLES("Print motorcycles"); private String ...


3

"A note on defining methods." This method has no added value whatsoever. All it does is calling another method with same method signature (with the first arg shifted as invokee). This could have been a good choice if either (1) you needed some validation (2) argument/result type parsing was involved (3) calling this method would have been a lot less code to ...


2

Implementing both Iterable and Iterator at the same time is a bit weird choice and while the API documentation for Iterator makes no claims about the implementation, I think most people would assume that subsequent calls to iterator() return a different object each time and those objects, if used for reading only, do not interfere with each other.


2

Ok, so, per @markspace, you've made this a lot more complicated than it has to be. In Python, yield is being used to save the state of a function. In Java, you'd just use a stateful object that executes the desired function. I'm pretty sure you could either create a FibonacciIterator or a FibonacciSupplier and meet your requirements. In either case, every ...


0

This program is an efficient one. I have added one more check-in if to get the square root of a number and check is it divisible or not if it's then its not a prime number. public static void main(String[] args) { Scanner sc = new Scanner(System.in); int T; // number of test cases T = sc.nextInt(); long[] number = new ...


1

use a recursive function I guess avoiding the stack-juggling (because of the recursivity of fromRPN) and the unnecessary creation of String objects (with StringJoiner) should make it fast enough. fromRPN( new ArrayList<String>( Arrays.asList( "1 3 + 2 4 5 - +".split( " " ) ) ) ); // ((1+3)/(2+(4-5))) public String fromRPN( ArrayList<String> ...


1

Simply call this code: DatabaseHelper db = new DatabaseHelper(this); db.OpenDB(); try { db.CreateTable(new myClass1()); db.CreateTable(new myClass2()); ... } catch (Exception e) { e.printStackTrace(); } Be aware that there are database upgrades which can't be automated so straightforwardly. E.g. the ...


Top 50 recent answers are included