4
\$\begingroup\$

I have written code to count the continuous ones in an array or you can use binary string for convenience. Any solution using the binary string is acceptable.

My Solution is:

#include<bits/stdc++.h>
using namespace std;
#define MAX 100000
int main()
{
  int n,q,k,count;
  string str;
  cin>>n>>q>>k;
  bitset<MAX>s,c;
  //inserting bits in array
  for(int i=0;i<n;i++)
  {
    int temp;
    cin>>temp;
    s[i]=temp;
   }
   cin>>str;
  for(int i=0;i<q;i++)
  {
     //making duplicate bitset
     c=s;
     if(str[i]=='?')
      {
         count=0;
         while(c!=0) 
         { 
            //using bitmask to count maximum no of continuous 1's-O(1's bit)
            c=(c&(c<<1)); 
            count++; 
          }
      if(count>k)
         cout<<k<<"\n";
      else
         cout<<count<<"\n";
   }
   else
   {
       //shifting each bit to right and updating first bit with previous last
      // bit
       bool lb=s[n-1];
       s=s>>1;
       s[n-1]=lb;
    }
  }
}
\$\endgroup\$
  • 4
    \$\begingroup\$ Try to give an example with possible value and invalid inputs and expected outputs. it will help people to figure out the way and the purpose of your programme. \$\endgroup\$ – Calak Nov 6 '18 at 14:52
10
\$\begingroup\$

<bits/stdc++.h> (like everything in your compiler's bits/ subtree) is not a standard header and therefore not portable. Even if you're willing to sacrifice portability, it's a poor choice, as it will slow compilation down compared to simply including what you use.

using namespace std; is poor practice. It makes your code less clear, and it may even silently change its meaning.

Don't use the preprocessor to name constants. Use a properly scoped, strongly typed C++ constant:

constexpr std::size_t max = 100000;

When using input streams, always check that operations succeed before using their results.

Variable names should be more descriptive. I have no idea what n, q, and k are supposed to be storing. In fact, these names are so useless that I gave up reading at this point - it's not at all clear what this is supposed to be doing.

\$\endgroup\$
  • \$\begingroup\$ i don't know how to do it. \$\endgroup\$ – Raja Babu Nov 6 '18 at 16:26
5
\$\begingroup\$
\$\endgroup\$
4
\$\begingroup\$

Don't declare multiple variables on a single line. it is error prone and more difficult to read.

int n,q,k,count;

should be:

int n;
int q;
int k;
int count;

Not sure what I mean by error prone?

int* n,q,k,count;

How many pointers do you have? one. only n would be a pointer in this declaration.


Let your operators breathe. The lack of whitespace makes your code harder to read.

for(int i = 0; i < n; i++)
  {
    int temp;
    cin >> temp;
    s[i] = temp;
  }

this is a little easier to distinguish.


Prefer prefix to postfix


Use more consistent indentation. I had to read the code three times just to realize that the scope braces didn't line up with each other. I almost flagged to close because

for ()
{
}
else
{}

would be broken, and that's how your braces line up horizontally.

\$\endgroup\$
  • \$\begingroup\$ FWIW, if * for pointer types are written before the variable name, then the declaration becomes int *n, q, k, count, which is less ambiguous. \$\endgroup\$ – Quelklef Nov 7 '18 at 1:26
  • 2
    \$\begingroup\$ @Quelklef yes but that is also often taught as a poor practice in C++. Regardless single line multi-variable declarations are harder to read. \$\endgroup\$ – brug Nov 7 '18 at 1:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.