Argument passing to the message() method in Report and MasterReport

[rkwright]

Errors and warnings etc. are passed to the Report (and MasterReport) class, which can then be sub-classed by clients to allow them to handle the error messages. In epubcheck 3 there were separate methods (error. warning, info, hint and exception). These are apparently deprecated (unused) but still present.

These methods have been replaced by (apparently) a single method, with the signature

message (MessageID id, MessageLocation loc, Object... args)

The problem is that args sometimes has location info, sometimes contextual info, etc. It varies and it is hard to tell what the info is. The MessageID is straightforward, as is the MessageLocation info. But unless one some divines what args is from the ID (very cumbersome) one is required to use some very convoluted code to handle the message properly.

Wouldn't it be simpler and more effective to pass a JSON string with name-value pairs? I see that epubcheck 4 included Jackson, so the library to read and write JSON is available. Making this change would make it much easier to handle the messages.

[rdeltour]

AFAICT, the varargs signature is used to pass objects which are intended to be used directly with a string formatter. Can you please elaborate on what you want to do with these objects other than that ? The previous methods in the 3.x branch only had a String parameter, so there doesn't seem to be any regression in terms of functionality.

In case we want to introduce name-value pairs, wouldn't a Map<String,Object> be more appropriate than a JSON string ? I believe this feature request would likely be postponed to the next major release.

[rkwright]

Yes, a map<> would work as well. JSON leapt to my mind because I have been doing more JS than Java lately, I guess. :-) I went back and reviewed my epubcheck 3 implementation of the tool and the same problem occurs there. Putting the name-value issue aside, the problem is that epubcheck, IMO, provides the wrong info. The MessageLocation has the fileName field as the name of the EPUB , NOT where the missing resource was referenced. I.e. it seems to me that the MessageLocation should return the name of the OPF file where the missing resource was referenced. It then also provides the name/path of the missing resource. But @mattgarrish pointed out in issue #501 the user doesn't really know where the error occurred. Most users can probably infer it must be in the OPF file, but it's harder for the tool to figure it out.

I think the name-value pairs would be a good help, but as you point out, that's too big a change at this point. But a relatively small change would address both this issue and #501 e.g. if the MessageLocation passed the OPF filename as the fileName and the missing resource as a var args, then a message such as a counterpart to OPF_003:

  OPF_003b="Item '%`1$s' was referenced in the OPF package file, but was not found in the EPUB"

Then if EpubCfiCheck would pass the OPF path/name in the MessageLocation and the missing resource as the args then I think it would address both mine and @mattgarrish concerns.