A job sequencing solution

Question

I have written this code as a college assignment fot the famous job sequencing problem. Please tell me any improvements if possible. Oh, for algorithms.h header file, refer my github repo

The code:

#include "algorithms.h"

struct Job {
    int index = 0;
    int time_slot = 0;
    int profit = 0;
public:
    Job() = delete;
    explicit Job(int i, int time, int pr) {
        index = i;
        time_slot = time;
        profit = pr;
    }
    bool operator>=(Job j) {
        if ((this->time_slot == j.time_slot && this->profit >= j.profit) || (this->time_slot > j.time_slot)) {
            return true;
        }
        return false;
    }
    bool operator<=(Job j) {
        if ((this->time_slot == j.time_slot && this->profit <= j.profit) || (this->time_slot < j.time_slot)) {
            return true;
        }
        return false;
    }
    bool operator<(Job j) {
        if ((this->time_slot == j.time_slot && this->profit < j.profit) || (this->time_slot < j.time_slot)) {
            return true;
        }
        return false;
    }
    bool operator>(Job j) {
        if ((this->time_slot == j.time_slot && this->profit > j.profit) || (this->time_slot < j.time_slot)) {
            return true;
        }
        return false;
    }
    bool operator==(Job j) {
        if (this->time_slot == j.time_slot) {
            return true;
        }
        return false;
    }
    friend std::ostream& operator<<(std::ostream& out, Job job) {
        out << job.index;
        return out;
    }
};

int main() {
    Job j1(1, 1, 3);
    Job j2(2, 3, 5);
    Job j3(3, 4, 20);
    Job j4(4, 3, 18);
    Job j5(5, 2, 1);
    Job j6(6, 1, 6);
    Job j7(7, 2, 30);
    std::vector<Job> vect = { j1,j2,j3,j4,j5,j6,j7 };
    vect = Sorter<Job>::mergeSort(vect);
    auto order = [&]() {
        auto it = vect.begin();
        while (it + 1 != vect.end()) {
            if (*it == *(it + 1)) {
                it = vect.erase(it);
                continue;
            }
            it++;
        }
    };
    order();
    std::for_each(vect.begin(), vect.end(), [](Job i) {std::cout << i << " "; });
    return 0;
}

Answer 1

The Job class has public members, so it's simpler to make it an aggregate:

struct Job {
    int index;
    int time_slot;
    int profit;
};

Then, use std::tie to simplify the comparison operators: (operands to these operators should be marked as const, which you didn't)

bool operator<(const Job& lhs, const Job& rhs)
{
    return std::tie(lhs.time_slot, lhs.profit) < std::tie(rhs.time_slot, rhs.profit);
}

// implement other operators in terms of <

You == is not consistent with other comparison operators, which is confusing and likely to cause problems.

Also note that if (condition) { return true; } return false; should be changed to return condition; for clarity.

This is convoluted:

Job j1(1, 1, 3);
Job j2(2, 3, 5);
Job j3(3, 4, 20);
Job j4(4, 3, 18);
Job j5(5, 2, 1);
Job j6(6, 1, 6);
Job j7(7, 2, 30);
std::vector<Job> vect = { j1,j2,j3,j4,j5,j6,j7 };

It should be simplified to

std::vector<Job> vect {
    {1, 1, 3}, {2, 3, 5}, // ...
};

This:

auto order = [&]() {
    auto it = vect.begin();
    while (it + 1 != vect.end()) {
        if (*it == *(it + 1)) {
            it = vect.erase(it);
            continue;
        }
        it++;
    }
};
order();

should be

vect.erase(std::unique(vect.begin(), vect.end()), vect.end());

(provided that I understand the code correctly).

And this:

std::for_each(vect.begin(), vect.end(), [](Job i) {std::cout << i << " "; });

is a complicated way of writing

for (const auto& job : vect) {
    std::cout << job << ' ';
}

or

std::copy(vect.begin(), vect.end(),
          std::ostream_iterator<Job>{std::cout, ' '});

Answer 2

1. public: is excess in a struct.
2. Members directly initialized in ctors are better directly initialized:

Job(int index, int time_slot, int profit)
    : index(index), time_slot(time_slot), profit(profit) {}

only more complex logic should go in ctor's body, if any.

3. As a matter of fact,

    if(cond) {
        return true;
    }
    return false;

is actually return cond;, indeed.

4. this-> simply increases the code size without any additional benefits.