Longest palindrome in an array

Question

I am new to programming, and think this code could be improved upon. Any suggestions?

  #include <stdio.h>
  #define size 10
  void main()
  { 

    int  i,j,ii,jj,arr[size]={1,2,4,2,1,1,2,1,1,2};
    int count=0, max_count=0, start_index=0, temp_start_index=0;

    for(i=0;i<size;++i)
    {
       for (j=size-1;j>=0;--j)
       {
          for (ii=i,jj=j;;++ii,--jj)
          {
             if (arr[ii]==arr[jj])
             {
                if (count==0)
                {
                   temp_start_index=ii;
                }
                ++count;
                if (ii==jj)    //odd number of elements in palindrome
                { 
                    count=count*2-1; 
                    break;
                }else{
                       if (ii>jj)   //even number of elements in palindrome
                       {
                          --count;
                          count=count*2;
                          break;
                       }
               }

           }

           else
           {
               count=0;
               break;
           }
       }


       if (count>max_count)
       {
           max_count=count;
           start_index=temp_start_index;
           count=0;
           temp_start_index=0;

       }
   }
 }

  for (i=start_index;i<start_index+max_count;++i)
  {
    printf("%d ",arr[i]);
  }
  printf("\n");
  printf("%d\n",max_count);
 }

Answer 1

Syntax & Styling

• Instead of #defineing size to be 10, you should declare a static const.

  static const size = 10;
  

  This answer explains nicely as to why. Good job on having this declared though, it's better than having magic numbers in your code.

• You should never declare main() to return void. There is no good reason to not declare it as int main(). See this answer for more explanation.

• You should declare your main with void parameters, as such:

  int main(void) 
  

• Your main() is doing too much within it. You should abstract your code into different functions and then call it from within main().

• Put the variable declarations to separate lines and initialize them to some value. From Code Complete, 2d Edition, p. 759:

  With statements on their own lines, the code reads from top to bottom, instead of top to bottom and left to right. When you’re looking for a specific line of code, your eye should be able to follow the left margin of the code. It shouldn’t have to dip into each and every line just because a single line might contain two statements.

• You should declare your for loops as such:

  for(int i=0; i < size; ++i)
  

  Note that this was introduced in the C99 standard. There is no reason you should not be using this standard in your code.

Algorithm

• Why do you limit yourself to finding palindromes within an int array? It's more typical to find palindromes within a string.

• Right now the time complexity for your algorithm is \$ O(n^3) \$, which is expensive. Let's use Manacher's algorithm which can do it in \$ O(n) \$. Here is a C implementation of the algorithm, but recognize that this has some bad practices within the code and should only be looked at to get an idea for implementing the algorithm yourself.

Answer 2

Not bad for a first attempt, but you want suggested improvements (the reasons are brief, search the net for better in depth explanations).

1. Hash define is OK in certain circumstances but is possible try and use static const size_t MaximumArraySize = 10; This will be type checked, it will show up in the debugger, its better is so many ways.
2. Never declare multiple variables on a line. It makes the code harder to read and less maintainable.
3. Always initialise a variable. Release configurations don't set the variables to default values.
4. loop variables should be defined in the for statement, as said above.
5. Spaces, there is no extra charge for pressing the space bar, always follow commas, semicolons and operators with a space.
6. COMMENTS - you know what it does today, you won't know next year. I'm not saying write an essay just a enough to explain anything slightly complex.
7. }else{ unless you are writing one of those new fangled web languages don't do this. For one reason there is no room to fit you comment in after else. But mainly because the { after the else doesn't line up.
8. count=count*2-1 using brackets can prevent you having to think. when you are up against a deadline and you reread the code bracketing it would stop it being read as count=count*1
9. "Variable names should be brief but meaningful", every Coding Standards document I have read has some variation of that statement. I appreciate its common practice to use i and j as loop variables, but its not always right. Have a think and see if you can come up with better names for the loop variables.

Sorry if any of that seems harsh, its not meant to be.