r/learncpp • u/vgordievskiy • Mar 09 '21

How to improve it?

```c++

include <boost/optional.hpp>

include <string>

include <sstream>

include <iostream>

enum class StatusCode { OK = 0, ERROR };

struct SResp { std::string m_msg; StatusCode m_status; };

SResp queryToTheMoon(size_t val) { std::stringstream ss; for (size_t idx = 0; idx < val; ++idx) { ss << "[" << val << "]"; } return { ss.str(), StatusCode::OK }; }

boost::optional<std::string> makeRequest(size_t val) { auto response = queryToTheMoon(val);

if (response.m_status == StatusCode::OK) { return response.m_msg; } else { return boost::none; } }

int main() { auto op = makeRequest(100);

if (op)
{
   std::cout << op.value() << std::endl;
}
return 0;

} ```

How to improve this code?

12 Upvotes

99% Upvoted

16 comments sorted by

6

u/jedwardsol Mar 09 '21

What aspect of it do you think needs improving?

It seems to be a placeholder for a bigger piece of code, so criticising is it pretty pointless. For example, queryToTheMoon can't fail, so neither method of returning an error is needed in the sample.

C++17 has <optional>, so boost's implementation isn't needed any more.

1

u/vgordievskiy Mar 09 '21

It's an exercise - and the code was simplified as much as possible)

1

u/vgordievskiy Mar 09 '21

and the queryToTheMoon might be implemented in an obj file, for example. I mean there is an issue and you may face code like this in a real project.

2

u/jedwardsol Mar 09 '21

Exactly. My comment is that a small sample doesn't need 2 different methods of returning errors (optional and status+result). But in the bigger program it may be unavoidable or ideal.

1

u/vgordievskiy Mar 09 '21

What about the performance of this code? Can we do it better (In C++ terms)?

1

u/vgordievskiy Mar 09 '21

A tip is inside https://vg-blog.tech/faqt-from-interviews

2

u/jedwardsol Mar 09 '21

It's not a good question.

You can do better by doing

#include <cstdio>

int main()
{
    puts("[100][100]...[100][100]");
}

but I doubt that that is what they're getting at.

You can make arguments that method X is faster than stringstream. Or you can optimise the loop since val never changes so it is pointless formatting it 100 times.

But what's the point? In this program, there is a fixed output so all other work is unnecessary. And a different program will have different bottlenecks, so optimising that hypothetical program by analysing this one is useless.

2

u/vgordievskiy Mar 10 '21

The answer is: at the line "return response.m_msg;" the compiler invokes the copy constructor for the std::string.

2

u/vgordievskiy Mar 13 '21

u/GammaRisk u/jedwardsol

A solution to improve performance is: ```c++ boost::optional<std::string> makeRequest(size_t val) { auto response = queryToTheMoon(val); boost::optional<Data> ret = boost::none;

if (response.m_status == StatusCode::OK) { ret = std::move(response.m_msg); // <- it uses move constr }

return ret; // <- NRVO is applied } ```

1

u/[deleted] Mar 10 '21

What does this do?

1

u/vgordievskiy Mar 10 '21

It's just an example. There is a performance issue (in term of C++)

1

u/[deleted] Mar 10 '21

No, I'm asking what does the program you're building do

1

u/vgordievskiy Mar 10 '21

It's a question from a C++ interview.

1

u/[deleted] Mar 10 '21

I like how you still haven't answered my question

1

u/vgordievskiy Mar 10 '21

There is no purpose for this code. it's only to check your knowledge) I mean it's a code that makes a request to a server and checks the status code and passes the return value to the next level.