Hackerrank's Queen's Attack II

Your Code:

1. Good on you to only include the headers you think you need. You don't use anything from <cmath> or <algorithm> though.

2. using namespace std; is plain evil. That namespace is not designed for inclusion, thus there is no comprehensive, fixed and reliable list of its contents.
   See "Why is “using namespace std;” considered bad practice?" for details.

3. Small trivial types are better passed by copy than by value. Less indirection means more efficient access, and no need to beware of anyone else mucking about with the value, which improves reasoning about the code and generally enables better optimisation.
   See "In C++, why shouldn't all function parameters be references?".

4. Take a look at std::span for passing a view of contiguous objects.
   See also "What is a “span” and when should I use one?".

5. C++ has for-range loops since C++11. Much less error-prone than manually fiddling with indices or iterators.

6. Using flags to check whether a ray encountered an obstacle and otherwise returning the maximum is distinctly sub-optimal. Just initialize with the maximum and update that as needed.

7. A std::vector of length two is a very poor data-structure to store a point's coordinates. It is very inefficient, inconvenient and error-prone. At least use a std::pair, std::array, or std::tuple, if you decline to invest a single line for a trivial custom type.

8. Your code never tests the user-input is well-formed. Actually, that can be justified for a challenge like this, so let's keep it.

9. return 0; is implicit for main() in C++ and C99+.

Your approach can be optimized and simplified further:

1. Use a queen-centric coordinate-system. All the checks are about the queen, and now much simpler.

2. If you store the reach of each arm of the queen's attack considering the obstacles you know (initialise with the border), you can immediately drop each obstacle after processing.

#include <algorithm>
#include <iostream>

int main() {
    int x, y, k, qx, qy;
    std::cin >> x >> k >> qx >> qy;

    int d = qy,
        l = qx,
        u = x + 1 - qy,
        r = x + 1 - qx;
    int dl = std::min(d, l),
        dr = std::min(d, r),
        ul = std::min(u, l),
        ur = std::min(u, r);
    auto update = [](int a, int& b, int& c){
        if (a < 0)
            b = std::min(b, -a);
        else
            c = std::min(c, a);
    };

    while (k--) {
        std::cin >> x >> y;
        x -= qx;
        y -= qy;
        if (!x)
            update(y, d, u);
        else if (!y)
            update(x, l, r);
        else if (x == y)
            update(x, dl, ur);
        else if (x == -y)
            update(x, ul, dr);
    }

    std::cout << (d + u + l + r + dl + dr + ul + ur - 8);
}

Beware: The above code was only proven right, never run.

General Observations

It was good that you included the necessary headers rather than using the catchall header provided by Hacker Rank. You did include headers that were unnecessary, the code compiles without cmath or algorithm. Only include what is necessary to make the code compile. Using unnecessary headers can increase build times because C++ actually creates a temporary file and copies the headers into that temporary file.

As a professional software developer one needs to be concerned with maintenance of the code (adding features, fixing bugs). You may write code but not be the person that maintains it because you could be on vacation, you may have gotten a better job at another company, you may have suddenly become rich.

This code will be very difficult to maintain. Some of it is very easy to read and some of it is almost unreadable. Some examples of almost unreadable code are:

    int u, d, l, r, ul, ur, dl, dr;                 //up, down, left, right, up-left, up-right, down-left, down-right
    u = d = l = r = ul = ur = dl = dr = 0;

and

    if (!modified[0]) u = n - qy;        //if they never been modified means no obstacles in path so use calculate distance from edge to queen (probably better way to do this)
    if (!modified[1]) d = qy - 1;
    if (!modified[2]) l = qx - 1;
    if (!modified[3]) r = n - qx;
    if (!modified[4] && qy != n && qx != 1) ul = (qx - 1 < n - qy) ? qx - 1 : n - qy;
    if (!modified[5] && qy != n && qx != n) ur = (n - qx < n - qy) ? n - qx : n - qy;
    if (!modified[6] && qy != 1 && qx != 1) dl = (qx - 1 < qy - 1) ? qx - 1 : qy - 1;
    if (!modified[7] && qy != 1 && qx != n) dr = (n - qx < qy - 1) ? n - qx : qy - 1;

The function queensAttack() is 88 lines and a single function that size is very difficult to write, read, debug or maintain.

Avoid using namespace std;

If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.

Complexity

The function queensAttack() is too complex (does too much). It should be broken up into functions, I see at least 3 possible functions and probably more. A good design technique is to keep breaking a problem into separate smaller problems until each problem is very easy to solve. This also make the code more maintainable.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

Magic Numbers

There are Magic Numbers in the queensAttack() function (0 through 7), it might be better to create symbolic constants for them to make the code more readable and easier to maintain, in this case an enum could also be used. These numbers may be used in many places and being able to change them by editing only one line makes maintenance easier.

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.

Prefer unsigned Types to Integer for Index Variables

When indexing into arrays or other container types it is better to use unsigned types such as size_t rather than integers. Unsigned types cannot become negative and using a negative index can lead to undefined behavior. The size() function of all container types returns size_t and the code is generating a type mismatch warning in the for loop:

    for (int i = 0; i < obstacles.size(); i++) {    //loop through all obstacles, if it is in path get distance to queen

Variable Declarations

Declare and initialize variables one per line. While the following results in a lot of added vertical space it is easier to read and maintain:

    int u = 0;
    int d = 0;
    int l = 0;
    int r = 0;
    int ul = 0;
    int ur = 0;
    int dl = 0;
    int dr = 0;
    bool modified[8] = { false };

If some needs to add or delete a variable it is much easier to add a line or delete a line than to modify the current code. In this case it might also be possible to have an array of directions that matches the array of modifications that already exists, especially if the enum mentioned above is used to index both arrays.

Variable Names

It is generally better to use descriptive variable names to make the code more readable. Comments are okay, but they need to be maintained as well, self documenting code is better than using comments when that can be done.

DRY Code

There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code multiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.

This is very repetitious code:

        if (abs(qy - obstacles[i][0]) == abs(qx - obstacles[i][1])) {   //diagonals, checking if it is on the diagonal path of the queen
            if (obstacles[i][0] > qy && obstacles[i][1] < qx) {         //check if it is top left diagonal
                temp = qx - obstacles[i][1] - 1;
                if (modified[4] && ul > temp || !modified[4]) {
                    ul = temp;
                }
                modified[4] = true;
            }
            if (obstacles[i][0] > qy && obstacles[i][1] > qx) {         //check if it is top right diagonal
                temp = obstacles[i][1] - qx - 1;
                if (modified[5] && ur > temp || !modified[5]) {
                    ur = temp;
                }
                modified[5] = true;
            }
            if (obstacles[i][0] < qy && obstacles[i][1] < qx) {         //check if it is bottom left diagonal
                temp = qx - obstacles[i][1] - 1;
                if (modified[6] && dl > temp || !modified[6]) {
                    dl = temp;
                }
                modified[6] = true;
            }
            if (obstacles[i][0] < qy && obstacles[i][1] > qx) {         //check if it is bottom right diagonal
                temp = obstacles[i][1] - qx - 1;
                if (modified[7] && dr > temp || !modified[7]) {
                    dr = temp;
                }
                modified[7] = true;
            }