Maintenance: Polish fqdncache.h

[yadij]

No description provided.

[rousskov]

Thank you for working on this cleanup.

[rousskov]

Please explicitly include ip/forward.h instead of starting to rely on current side effects of dns/forward.h inclusion (to get a forward declaration of Ip::Address).

Suggested change

	#include "dns/forward.h"
	#include "dns/forward.h"
	#include "ip/forward.h"

[yadij]

dns/forward.h includes ip/forward.h , no need to include it twice.

[rousskov]

dns/forward.h includes ip/forward.h

Yes, my change request is based on this fact.

no need to include it twice.

To get the current code compiled, we do not need to include it explicitly (as already acknowledged in my change request).

To avoid relying on side effects, we do need to include it explicitly (as already stated in my change request). This reason is enough to include ip/forward.h explicitly despite the current code compiling without that explicit inclusion.

[yadij]

And yet in #1848 (comment) you requested we rely on a side-effect of including the namespace Store forward declarations to also provide a global type.

[rousskov]

And yet in #1848 (comment) you requested we rely on a side-effect of including the namespace Store forward declarations

I did not:

• That change request is about using a reliable/direct/intended effect of store/forward.h header -- import of Store forward declarations. A foo/forward.h header exists exactly for that purpose -- it provides Foo forward declarations.
• This change request is about avoiding current side effects of dns/forward.h header. A foo/forward.h header is not meant to be used for Bar forward declarations (and may not provide them or stop providing them).

[yadij]

I think your definition of "side-effects" in this case is wrongly applied. The dns/forward.h provides the declarations for libdns API. The storage type(s) used for DNS responses (eg. Ip::Address) are part of that and always will be - the definition(s) will be provided via dns/forward.h.

[rousskov]

The dns/forward.h provides the declarations for libdns API.

dns/forward.h should provide forward declarations for libdns API. Today, it also provides three function declarations, but that (hopefully temporary) violation is not to be relied on when making design decisions (not to mention relying on side effects of that violation!).

The storage type(s) used for DNS responses (eg. Ip::Address) are part of that and always will be

Ip::Address is not a part of libdns forward declarations today, and even if it becomes a part of them tomorrow, it is not something libdns users should rely on.

The relevant rule of thumb is quite simple: If header B.h uses a name declared in header A.h, then B.h should include either A/forward.h (if sufficient to get B.h compiled) or header A.h itself (otherwise).

[rousskov]

dns/forward.h includes ip/forward.h

Yes, my change request is based on this fact.

no need to include it twice.

To get the current code compiled, we do not need to include it explicitly (as already acknowledged in my change request).

To avoid relying on side effects, we do need to include it explicitly (as already stated in my change request). This reason is enough to include ip/forward.h explicitly despite the current code compiling without that explicit inclusion.