1
\$\begingroup\$

I have to read console input and store data in the vector of structs. In case of any data format violation I have to print "Malformed Input" and return 1

Data format:

1 The first line is "#job_id,runtime_in_seconds,next_job_id"

2 The next lines are three comma separated integers (number of lines can be different)

Example:

#job_id,runtime_in_seconds,next_job_id
1,60,23
2,23,3
3,12,0
23,30,0

Could you please advise how to write better code in terms of time performance?

I was thinking about scanf("%d,%d,%d", &arg1, &arg2, &arg3) but not sure that it's C++ style

My code:

#include <iostream>
#include <optional>
#include <vector>


using namespace std;


struct Job {
    int job_id;
    int runtime;
    int next_job_id;
};


std::optional<Job> make_job(const string& s) {
    int job_id = 0, runtime = 0, next_job_id = 0;
    int i = 0;
    
    while (i < s.size() and s[i] >= '0' and s[i] <= '9') {
        job_id = job_id * 10 + (s[i] - '0');
        ++i;
    }
    if (i == s.size() or s[i] != ',') {
        return nullopt;
    }
    ++i;
    
    while (i < s.size() and s[i] >= '0' and s[i] <= '9') {
        runtime = runtime * 10 + (s[i] - '0');
        ++i;
    }
    if (i == s.size() or s[i] != ',') {
        return nullopt;
    }
    ++i;
    
    while (i < s.size() and s[i] >= '0' and s[i] <= '9') {
        next_job_id = next_job_id * 10 + (s[i] - '0');
        ++i;
    }
    if (i != s.size()) {
        return nullopt;
    }
    return Job {job_id, runtime, next_job_id};
}


int main() {
    string line;
    getline(cin, line);
    if (line.compare("#job_id,runtime_in_seconds,next_job_id")) {
        cout << "Malformed Input";
        return 1;
    }

    vector<Job> jobs;
    while (getline(cin, line)) {
        const auto job = make_job(line);
        if (not job) {
            cout << "Malformed Input";
            return 1;
        }
        jobs.push_back(*job);
    }
    // process Jobs
    return 0;
}
        
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

This is quite complex. A much simpler way would be to use the stream functionality:

std::optional<Job> make_job(std::stringstream s) {
    Job job;

    s >> job.job_id;
    if (s.get() != ',')
        return std::nullopt;

    s >> job.runtime;
    if (s.get() != ',')
        return std::nullopt;

    s >> job.next_job_id;
    // check that we have reached the end without any errors
    if (!s.eof() || !s)
        return std::nullopt;

    return job;
}

This might involve a copy of the string into the stringstream, but since C++23 std::stringstream you can move std::strings into it, or you could use std::spanstream. Alternatively, you could just pass the input stream directly to make_job():

std::optional<Job> make_job(std::istream& s) {
    …
    // check that we have reached the end of the line without any errors
    if (s.get() != '\n' || !s)
        return std::nullopt;
    …
}

Although reading the file explictly line-by-line is probably a good thing to do.

If reading a malformed job would be an exceptional event, consider just throwing an exception instead of using std::optional. This will simplify the code:

Job make_job(std::stringstream s) {
    …
    // check that we have reached the end of the line without any errors
    if (!s.eof() || !s)
        throw std::runtime_error("Malformed input");
    …
}

int main() {
    …
    while (std::getline(std::cin, line)) {
        jobs.push_back(make_job(line));
    }
    …
}

Also ensure you check that you have encountered the end of the file after the while-loop finished; std::getline() returns false also in case of errors, and you don't want to ignore those. So:

while (std::getline(std::cin, line)) {
    …
}

if (!std::cin.eof()) {
    std::cerr << "Error reading input\n";
    return EXIT_FAILURE;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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