gigasecond/gigasecond_test.cpp include header?

[lkskstlr]

This is a question:

Wouldn't it be nicer to include "boost/date_time/posix_time/posix_time.hpp" into the gigasecond_test.cpp as it actually needs it, instead of implicitly relying on gigasecond.h to include it. Maybe one wants to keep the header slim and thus forward declares boost::posix_time::ptime and includes the full boost header in the cpp? Additionally there is "boost/date_time/posix_time/posix_time_types.hpp" which has no i/o and would be sufficient for the implementation of advance() but then the test doesn't compile.

Please correct me if I am wrong :)

Cheers, Lukas

[arcuru]

Actually there was just a discussion about this exact issue. See #142 which has my opinion on it, though I'm open to discussing.

Also, #139 and #146 touch on gigasecond as well.

[lkskstlr]

Thanks for the references. Should have checked myself before opening an issue. I think that a test should not rely on the solution header to include another header which is actually needed by the test itself.
If posix_time.hpp was very large, it would actually be bad software design to include it in gigasecond.h only to use a type that can be forward declared easily. On the other hand this exercise is at the beginning of the track and the users probably don't think about such things anyway.
I understand the point that giving the user the header is kind of the solution already, but I think that good design practice is more important. I am however definitely no expert on this.

As you stated in #121 it would really make sense to be consistent on this for all exercises.

If you are interested I might have time to do this for all exercises in about 2-3 weeks. But I don't think this is a big issue right now. What do you think?

[Gamecock]

I have a related issue, not sure if it's just me or an actual issue.

/Users/Finch/projects/exercism/cpp/gigasecond/gigasecond_test.cpp:23:26: error:
use of undeclared identifier 'time_from_string'```

I can fix it by adding this header to the test case. #include <boost/date_time.hpp>
The header belongs in the test case, not the implementation. I dont understand, because it looks like it's part of posix_time time_from_string.
Let me know if:

1. I should fix on this issue.
2. I should create a new issue.
3. I'm doing something else wrong and should not touch the repo.

Thanks

[Gamecock]

Answer is 3.

I had included #include <boost/date_time/posix_time/posix_time_types.hpp> becuase that was all I needed in my implementataion, but the tests needed #include "boost/date_time/posix_time/posix_time.hpp" for time_from_string

[lkskstlr]

I think this actually is a reason why proper headers should be included? @patricksjackson ?

[arcuru]

I'm merging in change #201 that fixes this