0
\$\begingroup\$

Question Link.

Binary Search and Two Pointers related question.

For the above-mentioned question, In other people's solutions, people get AC with a binary search solution. However, I get TLE with Binary Search Solution. There might be some error in implementation and there aren't any Wrong Answers, so I can't figure out what's wrong with my solution. Other than that, I can't quite fathom the O(NM) time complexity solution which is mentioned in the editorial.

Also, what is the time complexity of this code per test case?

Any sort of help is appreciated. Thanks.

#include <bits/stdc++.h>
using namespace std;  
   
#define f(i,a,b) for(int (i) = (a); (i) < (b); ++(i))  
#define ull unsigned long long int
#define test int t;cin>>t;while(t--)
#define input(n) int (n);cin>>(n);
#define nfs ios_base::sync_with_stdio(false);cin.tie(NULL);cout.tie(NULL);
#define outl(n) cout<<(n)<<'\n';


int avg(vector<vector<ull>> v, int x,int y,int l){
    ull sum=(v[x+l][y+l]+v[x-1][y-1]-v[x-1][y+l]-v[x+l][y-1]);
    sum=sum/((l+1)*(l+1));
    return sum;
}

int main(void){
    nfs //macro
    test //macro
    {
        // used input macro
        input(n)
        input(m)
        input(k)
        vector<vector<ull>> prefix;
        f(i,0,n+1){
            vector<ull> r;
            f(j,0,m+1){
                if(i!=0 && j!=0){
                    input(x)
                    r.push_back(x);
                }
                else{
                    r.push_back(0);
                }
            }
            prefix.push_back(r);
        }
        // rowAdd
        f(i,1,n+1){
            f(j,1,m+1){
                prefix[i][j]+=prefix[i][j-1];
            }
        }
        // columnAdd
        f(i,1,m+1){
            f(j,1,n+1){
                prefix[j][i]+=prefix[j-1][i];
            }
        }

        // n*m*log(min(m,n)) loop
        ull count=0;
        f(i,1,n+1){
            f(j,1,m+1){
                int gap = min(m-j,n-i);
                int L=0,R=gap;
                if(avg(prefix,i,j,R)<k){
                    count+=0;
                    continue;
                }
                if(avg(prefix,i,j,0)>=k){
                    count+=gap+1;
                    continue;
                }
                while(L<R){
                    int mid=L+(R-L)/2;
                    ull avrg=avg(prefix,i,j,mid);
                    if(avrg>=k)R=mid;
                    else L=mid+1;
                }
                count+=gap-L+1;
            }
        }
        cout<<(count)<<'\n';
    }
    return 0;
} 
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Please heed How do I ask a Good Question? and a) improve the title b) include a sketch of the purpose of the code presented. \$\endgroup\$ Commented Sep 9, 2022 at 6:02
  • 3
    \$\begingroup\$ (Oh, look, single letter syntactic acid.) \$\endgroup\$ Commented Sep 9, 2022 at 6:04

1 Answer 1

2
\$\begingroup\$

This code is quite terrible to read. Please think about what your goal is when you do these code challenges. Is it just to get some site to say "AC" instead of "TLE"? Or is it to learn how to become a better programmer in general, so you will be able to work effectively on larger projects, and perhaps get a good job as a programmer? I'll go over all the issues below.

Include the proper header files

<bits/stdc++.h> is a non-standard header file. It may work with your particular compiler, but if you want to write portable C++ code, don't use it. Instead, #include the appropriate header files for all the standard library features you are using. For your code, you just need these:

#include <algorithm>
#include <iostream>
#include <vector>

Avoid macros as much as possible

Macros introduce all sorts of problems, but also importantly for you: they don't give you any increase in performance. The way you wrote macros also makes the code very hard to read for both humans and machines; I think that the highlighter in many code editors will be confused, as now you are writing statements that don't end with a semicolon. I suggest you just remove all the macros here.

For long type names, like unsigned long long int, you could use using to create a type alias. But I still would recommend not doing that; instead use one of the fixed-width integer types, like uint32_t. The problem statement says no value will be larger than \$10^9\$.

If you don't like typing a lot, check if your code editor can help you out. Most of them allow you to define macros in the editor itself.

Don't abbreviate too much

Single letter function/variable/macro names are easy to type but are hard to read. Consider f(i,0,n+1): what does f do? What are i and n? I now have to go look at the rest of the code just to figure out what this one line is meant to do. If, as in your case, most of your code is written like that, it really becomes unreadable. Try to give concise but meaningful names to things.

(I also have never heard of anyone getting a "code size exceeded" error on code challenge sites. However, if optimizing code size is your goal, then have a look at Code Golf.)

Don't store data unnecessarily

Your prefix stores (n + 1) * (m + 1) elements, with the top row and left column being filled with zeroes. This wastes memory. If you know those elements are zero anyway, there is no need to store them.

Use one-dimensional vectors to store matrices

This issue is about performance. A std::vector<std::vector<...>> is an inefficient data structure, as a std::vector might have to (re)allocate memory whenever something is added to it. For a single std::vector, it is not an issue, but now you have that overhead for every row of the matrix. Furthermore, the elements of the matrix are now also not consecutive in memory, which could impact performance as well.

If you know you have an n by m matrix, allocate a single vector of n * m elements, then access those elements using prefix[i * m + j].

If you also know up front how large a vector will be, call .reserve() so it can allocate all the memory it needs in one go.

Combine loops where possible

First you read in all the elements of a matrix, then you do "rowAdd", then "columnAdd". This means you loop over all the elements three times. However, you can do all of this in one go while you read the input: keep a sum of the elements you have seen so far in a row, and add that plus the value of the corresponding row above to the current row:

std::vector<uint32_t> prefix;
prefix.reserve(rows * columns);

for (uint32_t row = 0; row < rows; ++row) {
    uint32_t sum = 0;

    for (uint32_t col = 0; col <= column; ++col) {
        uint32_t value;
        std::cin >> value;
        sum += value;

        if (row > 0) {
            prefix.push_back(sum + prefix[col + (row - 1) * cols]);
        } else {
            prefix.push_back(sum);
        }
    }
}

Avoid unnecessary calculations

When trying to count the number of worth submatrices, you use bisection for the inner loop, which is good. But the outer two loops go over all the rows and columns. However, you can do better than that. Just consider getting a matrix as input where value of the bottom right element is less than k: in that case you know there possibly cannot be any worthy submatrix. Conversely, if the top left element is k or larger, you know that all submatrices are worthy. Therefore, by first finding out which part of the input matrix is k or larger, which you can do while you are reading the input, you can reduce the amount of submatrices you have to check.

\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.