# Tag Info

142

Note: At some point, this review drifted into the realm of assembler and GMP. An actual review is at the end of this post, whereas the first section discusses the runtime-problems concerning pow, wrong data types and arbitrary large integers. No life time for run time Would there be any way (on my current machine) to get this to run in my lifetime? ...

88

The biggest problem with using a for-loop to do this is that you are wasting CPU power. When using sleep, the CPU can, in a sense, take a break (hence the name "sleep") from executing your program. This means that the CPU will be able to run other programs that have meaningful work to do while your program waits. But in the for-loop the CPU continuously ...

77

That's not a bad program, despite the number of suggestions I'm about to make. I do recognize you're a learner, and a young learner at that. There are several levels at which we can analyze this program. One is checking the current implementation. Another is considering whether it is portable. We can question whether the interface is good — should ...

76

Well let's start with this: for (a = 1; a < 100000; a++) { for (b = 1; b < 300000; b++) { for (c = 1; c < 500000; c++) { let's ignore d for now. What do you do here? You check 1, 1, 1, then 1, 1, 2, then 1, 1, 3, ... up to 1, 1, 499999. Then you start over at 1, 2, 1. But you already checked 1, 1, 2, so why are you checking 1, 2, 1? ...

65

The code leaves a bad first impression because of how it is formatted. In particular, the extra whitespace between the return type and the function name, and that between the variable's type and its name, looks odd. Unless you are writing code that needs to be compatible with pre-C99 compilers, you should break the habit of declaring variables at the top of ...

56

Each blood factor can be present or not present. The blood types can be bit-encoded using one bit for the A factor, one bit for the B factor, and one bit for the Rh factor. enum { A=1, B=2, Rh=4 }; You would parse the blood type "AB+" as A + B + Rh == 7, and "O-" as 0 because it does not contain any A factor, B factor, or Rh factor. Survival requires the ...

47

If you're to implement something like this, you should first learn about how these things are done. I hope this doesn't sound too harsh. To explain it better, here is my variant of your code, with comparison to what C computes as e using expl(1): #include <stdio.h> #include <math.h> int main () { long double n = 0, f = 1; int i; for ...

47

This is a pretty reasonable start on a simple interpreter. Edward's suggestions are all good; a few additional suggestions: interpret("+++++++++++++[->.... Please break up that long line. C allows you to break up literal strings "like " "this." void goToLoopEnd(char** ip) { ... void goToLoopStart(char** ip) { ... If you wrote these instead as char *...

45

It's not possible to avoid undefined behaviour by testing for it after the fact! If the addition overflows then there is already undefined behaviour here: sum = a + b; so attempting to test afterwards is too late. You have to test for possible overflow before you do a signed addition. (If you're puzzled by this, read Dietz et al. (2012), "Understanding ...

43

I see some things that you might want to use to improve your code. Use an early bailout If the passed number x is less than 9, the routine can immediately return 0. Eliminate multiples of 2 Since 9 and 2 have no common factors, you can speed up the operation (on average) by shifting the incoming x to the right until the least significant bit is non-zero. ...

43

Implementation: As of now, the code is not practical, because the limit is 500 chars including zero termination. It does unnecessary copying. You need to determine the actual length of the string by relying on the fact that C strings are null terminated. size_t length = 0; while (*(str + length) != 0) { ++length; } Notice that I'm using size_t, ...

37

I see some things that I think could help you improve your code. Decompose your program into functions All of the logic here is in main in one rather long and dense chunk of code. It would be better to decompose this into separate functions. Check return values for errors The call to scanf can fail. You must check the return values to make sure they ...

35

Back in the day, home computers used to have a “turbo” button, which (when activated), made the processor slower. The problem was that games were coded around an input loop that was expected to take a specific number of milliseconds. If the loop was executed faster, the whole game would speed up, becoming unplayable. Today, computers have vastly different ...

33

Too frenetic I ran your program but it was very frenetic. It was constantly clearing the "Loading" prompt and reprinting it which resulted in a flickering effect. In addition, the cursor also moved around in a flickery manner (similar to the green box in the animated image). To improve this, I would do two things: Don't constantly draw when nothing has ...

33

printf("|---------- PROGRAM FOR AMICABLE NUMBERS.----------|"); This kind of banner message makes it harder to use the output of your program in a pipeline. I'd suggest removing it (the user has chosen to run it; trust them to know what they're doing!). int num1,num2,sum=0; What's sum for? It doesn't seem to be used. for(num1=1; num1&...

31

All of the answers given (when this 'answer' was first written) ignore an important constraint: native types. In C, a long int is a 32-bit (or greater) signed type, meaning the largest positive value that can be (counted upon to be) represented is $2^{31}-1$. The largest possible input to a function which calculates a fourth-power and [[can be counted on ...

31

I'm not able to comment on vnp's solution, but vnp was right the first time: you can brute force it in $O(n^2\log(n))$ time and $O(n)$ space. You don't need $O(n^2)$ space because you don't have to store the whole list of $a^4+b^4$ or $d^4-c^4$ upfront. Instead you only need to be able to list the values of $a^4+b^4$ and $d^4-c^4$ in ascending ...

31

Since you already have a std::vector<std::string>, it's much simpler to let that own the memory, and build a parallel std::vector<char*> which just keeps pointers into the original strings. The only difficulty is ensuring this isn't used after the owning vector goes out of scope. The simplest implementation is something like: std::vector<...

30

You could move the duplicated lines into a function, something like this should work: #include <stdio.h> float get_input(void) { float input; printf("Enter a distance in inches (0 to quit): "); scanf("%f", &input); return input; } int main() { float inches, floatFeet, input; int feet; while (input = get_input()) ...

30

Roughly 50% of all programming is about dealing with errors. You say your code works, but it doesn't handle errors and therefore doesn't work. Example 1: How many feet of steel was received today: four Program doesn't say "Error, try again". Instead it displays all prompts without waiting for any user input and exits. Example 2: How many feet of steel ...

30

Indeed you must have been working on this project for a while, since I answered a question on SO related to one of your old versions a while back. At least some of the feedback I had is still pertinent even now. Code and style comments #include <stdio.h> #include <stdlib.h> #include <math.h> #include <string.h> #include <time.h&...

30

Buffer overflow char array[MAX]; scanf("%s", array); This can overflow the buffer in array if the user input exceeds MAX-1 characters. There are multiple ways around this: Ask scanf to allocate an appropriately sized buffer (if you have a moderately recent glibc): char *array = NULL; scanf("%ms", &array); You need to release the allocated buffer ...

30

Some interesting things no one have mentioned about this, but it is an improvement when looking for the smallest solution: If the $\gcd(a,b,c) \neq 1$ then $a^4$,$b^4$,$c^4$ and $d^4$ are all divisible by that gcd to the 4th power, giving a smaller solution. Therefore, at least one of $a$,$b$, and $c$ must be odd, since if they are all even ...

29

Your find_second() function is rather weird. It never finds the second-largest number if it is a. It sometimes finds the second-largest number if it is b or c. I don't know if your program works or not — your main() tries to make up for the deficiencies in find_second() by calling it three times — but the function name is a big fat lie. Special numbers ...

29

Bug Winning a 6/49 game is, of course, unlikely. The probability of any single ticket having all six numbers correct is $$\dfrac{1}{\binom{49}{6}} = \dfrac{6!\,(49-6)!}{49!} = \dfrac{1}{13983816}$$ But your code required 1.5×109 draws to produce a win, which is 100 times more than the expected 1.4×107 draws. Why? Because your comparison loop… // ...

28

Yes, there is a better way: int CountOnesFromInteger(unsigned int value) { int count; for (count = 0; value != 0; count++, value &= value-1); return count; } The code relies on the fact that the expression x &= x-1; removes the rightmost bit from x that is set. We keep doing so until no more 1's are removed. This technique is described ...

28

You are doing brute-force trial division, and not particularly smartly. For example, you only need to test odd candidate factors up to sqrt(number). When you want to find many prime numbers, a much better algorithm to use is the sieve-of-eratosthenes. It involves just addition, no division, and skips processing of numbers that are already known to be ...

28

Typo lenght is spelled length. Magic numbers What does 95 signify? You'll want to put this in a named #define or a const. Allocation failure After calling malloc, always check that you've been given a non-null pointer. Allocation failure does happen in real life. Indentation You'll want to run this through an autoformatter, because your if block has ...

28

Your code is way too complicated. If any of A, B, or + is present in the donor, it must also be present in the recipient. That's all you need. bool is_compatible(const char* donor, const char* recipient) { if(strstr(donor, "A") != NULL && strstr(recipient, "A") == NULL) return false; if(strstr(donor, "B") != NULL && strstr(...

Only top voted, non community-wiki answers of a minimum length are eligible