add Status data structure

[zhjwpku]

This is adapted from Apache Arrow, which a pre-step before integrating the Result data structure.

[pitrou]

Also @zhjwpku can you make sure you proof-read your PR description? There are spell checkers that can help.

[zhjwpku]

Also @zhjwpku can you make sure you proof-read your PR description? There are spell checkers that can help.

Sorry about the inconvenience, will be careful in the future.

[gaborkaszab]

I think this PR makes too much assumptions that we will go with everything that the Arrow community chose. For instance this seems to decide the "exceptions vs status codes" question. Also decides which test framework to use, and additionally pulls in a number of macros too.
I think at this point we should start conversations on these topics, maybe on the Slack channel or as github issues.

I myself planned to come up with the headers for basic classes like Catalog Table and such, but I'd personally prefer exceptions over status codes for many reasons. Method chaining is something I find really powerful in the Java implementation but with status codes you wouldn't be able to do that if you have to check the status for every function call. Not to mention that constantly checking for statuses would make the code less readable. Also Statuses would make the parameter list longer, not to mention that lines would be longer (which somewhat matters now that we went for 90 chars).

Long story short I'm -1 for this PR.

[zhjwpku]

I think this PR makes too much assumptions that we will go with everything that the Arrow community chose. For instance this seems to decide the "exceptions vs status codes" question. Also decides which test framework to use, and additionally pulls in a number of macros too. I think at this point we should start conversations on these topics, maybe on the Slack channel or as github issues.

I myself planned to come up with the headers for basic classes like Catalog Table and such, but I'd personally prefer exceptions over status codes for many reasons. Method chaining is something I find really powerful in the Java implementation but with status codes you wouldn't be able to do that if you have to check the status for every function call. Not to mention that constantly checking for statuses would make the code less readable. Also Statuses would make the parameter list longer, not to mention that lines would be longer (which somewhat matters now that we went for 90 chars).

Long story short I'm -1 for this PR.

This PR is not trying to decide which test framework to use, I have started a issue #12 where we can discuss the test framework.

[zhjwpku]

I think this PR makes too much assumptions that we will go with everything that the Arrow community chose. For instance this seems to decide the "exceptions vs status codes" question. Also decides which test framework to use, and additionally pulls in a number of macros too. I think at this point we should start conversations on these topics, maybe on the Slack channel or as github issues.

I myself planned to come up with the headers for basic classes like Catalog Table and such, but I'd personally prefer exceptions over status codes for many reasons. Method chaining is something I find really powerful in the Java implementation but with status codes you wouldn't be able to do that if you have to check the status for every function call. Not to mention that constantly checking for statuses would make the code less readable. Also Statuses would make the parameter list longer, not to mention that lines would be longer (which somewhat matters now that we went for 90 chars).

Long story short I'm -1 for this PR.

I have seen some use cases of Method chaining in C++ world, mostly people use it for constructing object I guess. Can you elaborate how we can use it with exception to make the apis easy to use?

I don't like method chaining, it makes debug more difficult, when you set a breakpoint on the method chain and exception happens, how do you know which call give out the exception?

I think it's generally good practice to always write very short and concise lines. Every line should just make one method call.

There is an old stackoverflow question which I think we can take a look, Method chaining (fluent interfacing) - why is it a good practice, or not?

[gaborkaszab]

There is an old stackoverflow question which I think we can take a look
There is a stackoverflow for everything and for the opposite too. I just googled for "Status codes vs exceptions" and it gives me the first 5 results supporting exceptions over status codes, I guess because the algorithm is biased by my personal preference it learned from me.

when you set a breakpoint on the method chain and exception happens, how do you know which call give out the exception?
This works well in Java, I don't think it would be any different in C++ world. If you get an exception I guess you can check the trace where it was thrown and then set your debugger accordingly.
Also method chaining doesn't mean that you write everything in a single line, you could break as you want.

I did a quick run through on this PR and my general opinion is this: This PR assumes that we simply want to pull in auxiliary stuff from the Arrow project and without questioning whether we'd need them here or not we just pull entire files into this project. This is pretty difficult to review TBH, and if someone is not from the Arrow project it's even more difficult because they have to figure out the initial motivation of each functionality of another project.

I think this should be the other way around. Let's identify some functionality that we'd like to develop in this project, like interface for catalogs let's say, and if we find something in other projects that could help then pull in only the required part. Or let's have a very simple implementation ourselves, we can still evolve it later on.

[wgtmac]

I agree with @gaborkaszab that it would be better to discuss a concrete API design (e.g. Table, FileIO, etc.) before introducing a full-functional status implementation. If we decide to go with status over exception, we can add a basic status class at first and make it powerful later on.

[zhjwpku]

I agree with @gaborkaszab that it would be better to discuss a concrete API design (e.g. Table, FileIO, etc.) before introducing a full-functional status implementation. If we decide to go with status over exception, we can add a basic status class at first and make it powerful later on.

Agreed, maybe we should schedule a sync meeting to discuss some of the issues.

[lidavidm]

I'm a bit late here but even if we want to use Status/Result, it may be better to go with a backport of std::expected to at least try to align with the STL.

---

Ah sorry, I see discussion moved to #14.

[wgtmac]

@lidavidm I was about to reply to this thread to revive the discussion :)

[zhjwpku]

I'm a bit late here but even if we want to use Status/Result, it may be better to go with a backport of std::expected to at least try to align with the STL.

Ah sorry, I see discussion moved to #14.

No worry, I totally agree your suggestion, I will work on this way in the following days.

[lidavidm]

I have used this before: https://github.com/TartanLlama/expected

I think my main complaint (this applies to Arrow too) is that the structure is a bit opaque in a debugger. Arrow ended up writing a GDB pretty printer script.

[zhjwpku]

This is obsoleted by #40, though we haven't decided to adopt expected or not, this PR can be closed. Thanks for all the inputs.