Formatter should define file extension

[npryce]

I have a formatter for Markdown output, another for CSV output, etc. etc.

I'd like the SourceOfApproval to create files with the extension .md when I use the Markdown formatter, .csv when I use the CSV formatter, and so on.

E.g. for the Markdown formatter, I'd like the files to be named BlahBlah.blah.approved.md & BlahBlah.blah.actual.md

[dmcg]

This would end up tightly coupling SourceOfApproval and Formatter, which will kill Approver.writeFormatted(object, aFormatter) and assertApproved(object, aFormatter) for not much gain.

You though can tightly couple your implementation with impunity. FileSystemSourceOfApproval.typeExtension() is protected - is there anything to stop you subclassing it to provide the behaviour you need?

[dmcg]

In fact, don't you know it at the point you create the SourceOfApproval?

public class Pearlfish {
    public static ApprovalsRule pearlfishApprovalsRule(final String srcRoot) {
        return new ApprovalsRule(new ApproverFactory() {
            public Approver create(String testName, Class<?> testClass) {
                TemplateFormatter formatter = new TemplateFormatter(testClass, testName, Charset.defaultCharset());
                FileSystemSourceOfApproval sourceOfApproval = new FileSystemSourceOfApproval(
                        new File(srcRoot), testClass.getPackage()).
                        withTypeExtension(formatter.extension());
                return new Approver(
                        testName,
                        sourceOfApproval,
                        formatter,
                        Reporters.reporter());
            }
        });
    }
}

[npryce]

I think you're assuming that the SourceOfApproval will take a file extension as a constructor parameter and hold it in a field.

I think it should take a file extension as a parameter of its methods. This would then work fine for assertApproved(object, aFormatter).

[npryce]

I can see where the Approver can pass the file extension from the formatter to the SourceOfApproval. But it would create a lot of duplication. It feels as if the SourceOfApproval is created too early. It should be a Curried Object that closes over some information about file naming (test name, file extension etc.) and a storage implementation.

Also, the Approval holds onto an output stream from the SourceOfApproval in a mutable variable, but that seems to conflict with passing a formatter to the assertApproved(...) method.

I think that the output stream is held so that the Approver can write a transcript. I think the Approver has too many responsibilities -- transcript writing should be done by another object that holds onto an output stream obtained from an Approver.