HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Longest palindrome in an array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arraypalindromelongest

Problem

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

#include 
  #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=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);
 }

Solution

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.

Code Snippets

static const size = 10;
int main(void)
for(int i=0; i < size; ++i)

Context

StackExchange Code Review Q#131130, answer score: 7

Revisions (0)

No revisions yet.