# Tag Info

41

for(int i=0; num_to_parse; i++) wut Why is num_to_parse your condition? Then you have if (i == num_to_parse) in there, too? That's odd. I'd rewrite this as: for(int i=0; i < num_to_parse; i++) { int point_end = num_to_parse - i - 1; if(inputString[i] == inputString[point_end]) { continue; } else { return false; } } return true; If you ...

36

Others have pointed out a fix you could make to your implementation. But there is a way to write this which is much simpler. That takes advantage of a simplified definition of palindrome: "a word which reads the same forwards and backwards". Now I haven't done PHP in years, and don't have a test environment, so this might have issues, but the basic idea ...

32

The other answers are all giving you different ways to reorganize your loops. They're all not bad in their own way, but the better solution in C# is to not write any loops in the first place. Your program could be written like this: static void Main() { var query = from first in ThreeDigitNumbers() from second in ThreeDigitNumbers() ...

25

I would prefer the first one for a few reasons. It's more readable and easily understandable in my opinion. There's only one string, so less overhead. (Which is negligible unless this is being called a lot.) There's only one array. Reverse creates a second array. That being said, an actual review is in order. Methods that return a boolean value should ...

25

A few notes: Put some include guards in your header. Name your header something more unique and appropriate. Always declare what parameters your function takes in, even if nothing. int main(void) You might wonder why we have to do this. Imagine we have the function foo() declared as such: int foo() In C, this is known as an identifier list and means ...

22

A special case length < 3 seems like a bug. A string aa is a palindrome, and one could successfully argue that a single-letter string is also a palindrome. A boolean isPalindrome is redundant, and forces the code to test the effectively same condition twice. An early return is perfectly fine here. A traditional idiomatic (and arguably more performant) way ...

20

Simplification I found that the multiple pointer solution you used was a bit hard to read and verify for correctness (although the function was correct). I would use array syntax and indices instead, like this: bool palindrome(const char *text) { for (int left=0, right=strlen(text)-1; left < right; left++, right--) { if (text[left] != text[...

20

It's enough to loop until word.length() / 2, as this will compare the first half with the second half, so no need to go until the end. As you use word.length() multiple times, you could extract it to a helper variable. There is a typo in the method name. As for testing, yes, JUnit is the way to go. In a cloud IDE, you could create a helper method that ...

19

Firstly, this is something I see a distressing amount in beginner Java programmers: removeSpace(word).compareTo(removeSpace(reversed)) == 0 There's a method for checking if two Strings are equal. It's called equals. It'd be used like this: removeSpace(word).equals(removeSpace(reversed)) If you wanna ignore case, you could use equalsIgnoreCase: ...

18

Here is the most common solution for this problem: public static bool IsPalindrome(string s) { char[] array = s.ToCharArray(); Array.Reverse(array); string backwards = new string(array); return s == backwards; } and if you are using Linq: public static bool IsPalindrome(string s) { return s == new string(s.Reverse().ToArray()); } ...

15

Complexity Your answer is $O(n^3)$ and not $O(n^2)$. You have three levels of nested loops... two inside the longestPalindrome() method and another inside the isPalindrome(). Can this problem be solved with less complexity? I believe it is possible to do an $O(n^2)$ solution ... I'll have to think it through though. Test-Cases Your code may have ...

15

Documentation You should definitely add a docstring. It took me a few seconds to figure out was was going. Make sure to add some examples in the docstring as well. Naming The name cache may not be the best name. Why? Let's look at the definition of cache: is a hardware or software component that stores data so future requests for that data can be ...

15

Ignoring issues of correctness and some of the other things that the other answers have gone into detail on, and instead focusing on simple program transformations (transforming the program into an equivalent program): It is worth pointing out that whenever you have if (c) { return true; } else { return false; } you can just replace that with return ...

15

There are some things to be said about your C version as well, but since you explicitly asked about the C++ version (and also because my C-knowledge is not that great), I will leave those for somebody else to comment on. General Hints and Tips You are passing possiblePalindrome by value where a pass by const& would be much more appropriate since you ...

14

For all practical purposes, recursion is a bad idea for this problem, mainly because String.substring() is an expensive operation, especially since Java 7. Let's assume that you're doing this just as a fun educational exercise. Single return statements are overrated. There's no practical advantage to forcing your code to have just one return for its own ...

14

Algorithm Your time and complexity problems are probably the more serious ones. Your code starts small (at 100), and then works up to 1000. This is not the most efficient way of brute-forcing the problem. A better solution would be to start at 999 and work back to 100, but limit it when the result becomes impossible.... let me explain: int max = Integer....

13

There are a few things you could optimize: If you (or the grading server?) are running this on Python 2.x, you should definitely use xrange instead of range (about 2 seconds on my system) In your prime sieve, you have to check the multiples only if the current number is a prime (down to 1s); also, instead of multiplying, count by multiples of is use a ...

13

I'd try to break some habits, and make others in their places. One habit to break is using while (!file.eof()) -- it's nearly always a problem (including this time). In its place, I'd consider initializing the array of input strings from a pair of std::istream_iterators. I'd try to form the habit of creating fstream objects initialized with the name of the ...

13

CODING STYLE For the beginning, some coding habits you seem to have picked up could/should be changed. Declare your variables before use, not at beginning of the method (does not apply for loops, where you deliberately want to declare variable before loop). This was, and still is, widely used in C/C++. Earlier was possible to declare new variables only at ...

13

Be aware that your palindrome function only works on strings where the characters are encoded as 1 byte per character. If you receive a multibyte encoded string in an encoding where characters are not always the same width, this method will not work. e.g. If you had a UTF-32 string and some type utf32_char* that worked on whole characters, your same method ...

13

I've seen a lot of answers here over-generalizing. You're dealing with a std::string, so you can just do: bool IsPalindrome(const std::string& s) { return std::equal(s.begin(), s.begin() + s.size() / 2, s.rbegin()); } You don't need anything fancy to find the midpoint. Just add the size/2. As others have mentioned, take the string by const std::...

13

To answer your comment, yes there are equivalent of PEP8 in java. I suggest checkstyle, this plugin works in lots of IDE / text editors and can be configured with your code style. Code review reverse_string method In java, we try to use the upper-case version of Snake case only on constants / enums. For the methods and variables, I suggest that you use ...

12

You are unnecessarily importing punctuation Avoid reassigning module names such as string in your code A list comprehension would be a simpler way of filtering out the data you don't want from string import ascii_letters def is_palindrome(candidate): """ Returns True if candidate is a palindrome, ignoring whitespaces and punctuation. """ ...

12

Things you did well You defined i within your for loops, and abided by the C99 standards. You used an external library instead of writing your (more likely inefficient) own methods, and thus avoided unnecessarily reinventing-the-wheel. Things you could improve on: Efficiency: Copying a whole number into a external array with gmp_sprintf() in your ...

12

I do not do C# but I can offer another way of approaching the problem: As our range of numbers is $[100\cdot100, 999\cdot999] = [10000, 998001]$, assume a palindromic number on the form $abccba$. The largest $abccba < 998001$ is $k = abccba = 997799$. We need to determine if $k$ can be written as the product of two 3-digit numbers. Using a ...

12

Comments on the code as it is This: __attribute__((fastcall)) will not improve your performance, as the culprit is the algorithm not the compiler. If you really want to tell the compiler to optimize your code harder, you can try inline instead. This forward declaration: extern bool isPal(const int &number) __attribute__((fastcall)); is ...

12

Bug Given the input "racebar", the function returns true. The problem is that your boolean variable is reset after each character pair, so you are only really testing the first character with the last. I would do the following: Get rid of the boolean variable and return false on the first mismatch. Fix the loop iteration to only iterate through ...

12

All other factors being equal, the best solution to a problem is the one that's easiest for someone else to understand. You mention that you don't want to use anything but "basic loops and mathematical operations," but code that is easiest to understand usually does not contain raw, explicit loops. Therefore I suggest the best solution is simply: bool ...

12

More tests Having a test suite is nice. It would be even nicer to make it more comprehensive by adding: edge cases (empty list for instance) inputs with an even number of elements negative test cases : input is not a palindrome. Here is a suggestion, I took this chance to remove the duplicated logic by extracting the data into a proper data structure: # ...

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