New answers tagged

1

If the objective is to avoid duplicating information in memory, the code you have given won't archive it. The new keyword hides the original value. It doesn't replace it. You should try another approximations, like the Decorator pattern: https://en.wikipedia.org/wiki/Decorator_pattern The MasterData inherits from the GlobalData class and stores internally ...


1

You're checking way too much in your program. In your test function, the all is going through every single element in the list and checking if it's True. What you should do is do the check yourself, and return on the very first instance of a False value. With the other optimizations made, this speed up runtime from 18 seconds to about four seconds. You can ...


1

Your current approach of using a nested loop to check every Person's distance from every other Person is O(n ^ 2), which is pretty expensive as the number of people increase. One alternative is to iterate over all infected people and add all coordinates within the infection radius to a Set, O(n). Then iterate over non-infected people and see if their ...


4

TLDR: it can be done much more quickly and much more concisely, in 28 minutes end-to-end, and in 12 minutes when all XML are already on disk. The key trick: using the long rather than wide format for each data frame. There are two parts to your code: style and performance. Let me post my approach and comment on why I coded it like this. For imports, I ...


0

Queues are not designed to be used like this. Your iterable_queue clearly shows that you are not using queues the way they are supposed to be used. And the checkValidity function should be simplified with std::is_sorted from the standard library. In q.empty() || q.size() <= 1, the first condition is subsumed by the second. The while(q.size()){ loop ...


3

Overall You don't use encapsulation. Which makes your list vulnerable to incorrect initialization and accidental incorrect modification from outside the list. You use several C based style choice rather than C++ style which make your codde harder to read. Code Review Only a list of int? class Node { public: int value; // int only ...


1

1. You never check if head was assigned a valid pointer before dereferencing it here: int find(struct Node *head, int n) { int count = 1; //if count equal too n return node->data if(count == n) return head->value; // <--- here //recursively decrease n and increase // head to next pointer return find(head->...


3

I see a number of things that may help you improve your program. Decompose the code into smaller functions This code is very dense, very long and not well organized making it difficult to follow and understand. As a first step, I'd recommend extracting out smaller functions, such as to calculate error values. Each function should be small, well ...


2

So, I am assuming that you are aware that a more efficient algorithm for finding prime numbers can be written, so we'll ignore this aspect. Comments to the prime_finder method If we pass num=1 to the method, will it return what you expect? Do you consider 1 a prime number? But even more important, the name of the method is misleading. The method does not ...


3

Adding to what @dariosicily mentioned already: Performance If you just need to find out a part-of-speech of each word and do not need to build a phrase-structure tree of sentences, you need only to specify 3 annotations (without parse): props.setProperty("annotators", "tokenize,ssplit,pos"); I assume this can give you a significant boost in performance. ...


4

Welcome to Code Review, here some suggestions about your code: public class review { ... } Java classnames always begin with uppercase letter so rename it to Review. private ArrayList<String> restaurants = new ArrayList<String>(); In java language it is preferable using if possible an interface like List on the left part of the assignment so ...


2

You've got inefficiency here: // Sum of a subarray, based on B(x, i, L) -- i is one-indexing public static double sum(double[] x, int i, int L) { return IntStream.range(i, i + L) .parallel() .mapToDouble(idx -> x[idx - 1]) .sum(); } I don't know how big L is, but ...


1

You can simplify this by rearranging the function as follows (writing z=(a+b)/2): a log(a/z) + b log(b/z) + (1-a) log( (1-a)/(1-z) ) + (1-b) log( (1-b)/(1-z) ) = a log(a) + b log(b) + (1-a) log(1-a) + (1-b) log(1-b) - 2z log(z) - 2(1-z)log(1-z) The first 4 terms can be evaluated outside the main loop, so that you only need to evaluate log(z) and log(1-z) ...


3

Avoid doing explicit appends in your for loop and use dictionary (or list) comprehension instead; this makes it run more than 3x faster on my machine. That is, do something like def run(file, content): data = etree.parse(file) get_path = lambda x: data.getpath(x) paths = list(map(get_path, data.getroot().getiterator())) content = [ ...


1

Here are some things that may help you improve your code. Fix the bug The code allocates memory for the passed arr_lengths but never returns a pointer to the newly allocated memory to the caller. The code currently has this: int* lengths = *arr_lengths; // more code (lengths) = malloc((max_dis+1) * sizeof(matrixOptimizationValue)); Instead it should be ...


3

Seems like it might be a good use for parallizing with the multiprocessing library. Something like this (untested): from multiprocessing import Pool # your other code goes here mypath = '/Users/marcelwieting/Documents/AllPublicXML' folder_all = os.listdir(mypath) def process_folder(folder): df_final = pd.DataFrame() mypath2 = mypath + "/" + ...


1

After applying the suggestions the function has changed as follows. static final double ln2; static final int twoExpLimit; static final double precision; static { int k = 0; int n = Integer.MAX_VALUE; while (n > 1) { n /= 2; k++; } twoExpLimit = k; // WolframAlpha ...


6

To speed it up a fraction more add double dIteration = aIteration/cIteration; and double d = a/c; before the loop. In the loop remove a and c and insert d*= dIteration; ln += d/b; You then have one multiplication, one division and an addition in the loop, cutting out two excess multiplications in your version. Edit: you can get further ...


6

Since it's always \$n^2\$ in the second version, by not using java.lang.Math#pow (\$n * n\$), you can save computation time. It takes me ±3k nanoseconds instead of the ±20k nanoseconds with the java.lang.Math#pow. static double lnApproximationOptimized(double x, int n) { double a = x - 1; double aIteration = a * a; double b = 1d; ...


6

Aside from the notes already given, here are two big ideas to think about: Don't repeat yourself (DRY) This is something you'll hear repeated a lot in discussions of code. Any time you see the same "magic values" repeated more than one place (e.g. 0 and 20), or you see two lines of code that do exactly the same thing for exactly the same reason (e.g. your ...


3

Now for something completely different: an implementation in C. This is somewhat simple and stupid, and for all intents and purposes it executes instantly. It does not have any hash maps or hash sets. It tracks, for each pet, a letter frequency counting array that is sparse - it technically tracks the whole ASCII-extended range for efficiency's sake. This ...


3

To add to @Sara J's answer, in Python, it's generally a good practice to wrap your main code (so the last two lines) in a if __name__ == '__main__': statement so your script can be either: Directly run Imported and its functions used as the dev that imported it pleases. https://stackoverflow.com/a/419185/1524913 Also, contrary to a lot of other ...


4

First off, some minor nitpicks on style. In Python, variables are usually given names like_this rather than likeThis - most of your names are fine, but userInput should probably be user_input instead. You usually want a space on each side of operators, guess = int(input()) is more pleasant to look at than guess=int(input()) Second, your program's behaviour ...


2

One way to speed this up is to use a package designed for numerical evaluation, numpy. It is implemented in C and can take advantage of multiple cores if necessary. The only limitation is that you can only create arrays that fit into memory. At least on my machine (16GB RAM) your current values fit easily, though (if each integer takes 64 bytes, then I could ...


2

The problem of your code is the way you generate triangles. You just create three independent random numbers. In theory, you could always end up rolling numbers that don't end up in a valid triangle and thus your code would not even terminate. That is, because triangle side lengths are dependent. To always create a valid triangle, you can instead roll two ...


3

Use The Rule Don't search for words that match the rule! You already know the rule. Use it to generate the BFF words. That is, start with a common pet and filter out all the words that aren't two letters longer or that don't have all the letters in the common pet. The result is a list of the BFF words for that pet. The non-BFF words are generated using ...


4

The easiest speed-up I can think of is a letter-counting pass. In other words: apply collections.Counter() to the word in question, and keep a pre-computed tuple of Counters for both pet types. The thing that's killing your performance is order - there are many, many, many re-ordered results from permutations, but they literally don't matter since you're ...


5

In the sort() function its currently hardcoded to merge sort which I assume is not intentional. However, if you do allow the user to give an input you should do some error checking. You'll currently get unexpected behaviour if the user enters anything other than 1 or 2. You should use std::swap rather than implementing your own swap function. Some types can ...


-2

READABILITY Function: int count(T* array) I don't prefer this way of acessing a (T) while(*(array+i)), because of readability reasons. Better way: while(array[i]) PERFORMANCE Function: void swap(T& t1, T& t2) Faster way is to use XOR (^) swap algorithm. it is faster and small amout of memory is used. if (t1 != t2) { *t1 ^= *t2; *t2 ^= ...


10

For each word in the lexicon you are searching through each email: (11,314 emails) * (60 words/email) * (211441 word lexicon) = lots of comparisons. Flip it around. Use collections.Counter. Get the unique words in each email (use a set()) and then and update the counter. from collections import Counter counts = Counter() for email in x_train: words ...


2

Your for loop can be reduced to one line, utilizing sum: frequency_train = [ sum(1 if lexicon_train[i] in email else 0 for email in X_train) for i in range(211441) ] It removes the need to create the initial list of zeros. For performance, I'm guessing the size of the lexicon and the number of iterations are slowing it down.


1

Using 1201ProgramAlarm's comment, the code ran slightly faster but was still slower than taking multiples of 3(effectively 6). However finding if the current composite number is in the multplicative group mod 30 seemed to be the one that took up a long time and could have been memoisation. This is done by precomputing how many times one adds the prime to ...


2

That's pretty solid an implementation, I can find only a few spots. Database connection First and foremost, never a Model should create a database connection. All right now you have only a single model. What if you'd decide to extend it for CDs? Magazines? Add users? How many simultaneous connections will be made from a single script instance? A model ...


3

Checking over 100 million possibilities is going to be slow in any language. That being said, here are some ways to speed up the code a bit. There is no need to actually get the list of all combinations when you only need them one at a time. Especially when you have 100 million of them this will be very big in memory. Instead just use it as the generator ...


3

One drawback of this implementation is that, when from and to are of different lengths, it works in quadratic time. Consider auto attack = "12324252627"; replace_all(attack, "2", "42"); How many times most characters are copied to only be copied or, even worse, replaced later? (Coincidentally, when pattern and replacement are of equal lengths this shouldn'...


6

This code is nicely written and clear in both code and description. Good job! I think there are still some things that might be improved. Fix the bug The updated value of start_pos is not being used for each iteration of the loop. Instead, it should be, so the while loop should be this: while ((start_pos = string.find(from, start_pos)) != std::string::...


2

Low Hanging Fruit I obtained a 27% speed-up with one tiny change. For this speed-up, I didn't want to wait 2 hours for tests to run, so I used a 3x20 board. Elapsed times: 0:01:44 - before the change 0:01:16 - after the change I'm sure you'd like to know what the change was. I can't drag it out much more, so here it is. I added this line to the Nodes ...


3

When the original multiples-of-3 code finds a prime, it starts setting bits with the square of that value (for (int j = i * i). Your multiples-of-30 code does not do this, and can waste a lot of time marking numbers "not prime" that have already been so marked. As the new prime gets larger, this will consume a growing amount of time.


3

Prime Generation As mentioned by vnp, use the Sieve of Eratosthenese. In that implementation, use a BitSet(1_000_000) for efficient memory usage during your sieve; a sieve for primes up to one million will only take 125 KB of memory. Keep the sieve around after you've generated your prime numbers, because it makes a very efficient \$O(1)\$ time complexity ...


2

Indentation This: def conversion(acqList): prefix = ['MCS','FT', 'FS', 'GNo', 'Rs', 'WHw', 'Cy'] newList = [] #to check if there is any of the items contains the prefix for e in acqList: is (as a surprise to me) valid syntax, but that doesn't mean it's a good idea. The beginning of a comment, when that comment is the only thing on the line, should ...


6

Generation of prime numbers is suboptimal. Use a sieve of Erathosthenes. isPrime is highly suboptimal. You already generated an array of all necessary primes, so just binary search it. Breaking the loop in if(sumFromTo(cumulativeSums,start,end)>limit) break; looks like a bug. The intention is to loop by decreasing end, yet since the ...


2

There isn't any need to name all your Sx variables and copy+paste the code to build each one; just build them in another loop. You can do both loops as list comprehensions very concisely: data = np.array([ [(r[i+s*N, 0] - rcom[s, 0])**2 for i in range(N)] for s in range(400) ])


3

Type hints """ Attributes: name(str): Activity Name iid(str): Activity ID, uniquely identifies the activity start(str): Start Time end(str): End Time parent() """ It's nice that you've documented these types, but it would be better to tell Python (or at least your IDE) about them: def __init__(self, name: str, ...


3

Introduce a variable for blip3_dump()'s offset expression: for (int b=0, e_size2b = e + size2b; b<3; ++b) { int offset = e_size2b + b * size_grid_3b; for (int d=0; d<3; ++d) { force_fp[i][d][offset] += f[d][0][b] + f[d][1][b]; force_fp[j][d][offset] -= f[d][0][b]; force_fp[k][d][offset] -= f[d][1]...


4

Invoke is a blocking call that returns only after that call has competed. That means your loop is also including the time it takes to marshal over to the GUI thread and complete. You probably don't want that. I would use BeginInvoke instead, which does not wait for the method to complete on the GUI thread. This is also the difference between ...


2

The storage of both points and queries is suboptimal - in both cases, they are plain unsorted arrays. We could use better strategies for representing one or both. As a simple example, consider keeping points as a list of rows in ascending order, with each row being a list of points. Now, when we evaluate a query, we can quickly skip the rows that are ...


2

There is no reason to use lists instead of arrays, new int[size] will give you an array full of 0s without any overhead. You can get away with creating only one array (leftMax) and then while computing the values of rightMax, instead of saving them into an array, complete the full calculation of how much to add to ans. Like this: public int ...


3

I'm interested in constructive feedback on how I could improve the algorithm to make it faster and do less checks. processQuery() is O(n) with for(size_t i = 0; i < npoints; i++) { An alternative would create a binary like tree in 2 dimensions. Not a BST, bit a quadtree. Then the searching within a rectangle could take advantage of potentially O log(...


5

Bug adjust_volume(0) presently results in: UnboundLocalError: local variable 'bid_volume' referenced before assignment since if position_size == 0: is currently indented inside the elif position_size < 0: statement, so is unreachable. (This could be a result of poor formatting when copying the code into the question post.) Possible Bug Some volume ...


4

Here's a better algorithm: It has been benchmarked and reduces the time from 29 hours to 1.1 seconds This is approximately 95,000 times faster. Edit: Faster version, described below, reduces the execution time to 0.68 seconds, which is 153,500 times faster. We only need to consider the number of each type of honor and then combinations of remaining spot ...


Top 50 recent answers are included