Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1
    • Fix Version/s: 1.2
    • Component/s: Validation
    • Labels:
      None

      Description

      Certain methods in TypeHandler print a message to stderr when they are unable to create the object they are to return. I don't think this should be. The documentation for each method clearly states that if it is unable to create the object, it returns null. If more information should be reported to the caller, these methods should be changed to throw an exception. I've removed the printing of these messages from TypeHandler.

      This fix also makes the unit test output clean. Right now, certain test cases exercise the "returns null" behavior of some of the TypeHandler methods, and this causes various error messages to be written to stderr which makes the test case output look unclean and like something has gone wrong when in fact everything is working correctly.

      Attached is a patch for this fix. The patch is against http://svn.apache.org/repos/asf/commons/proper/cli/branches/cli-1.x.

      1. CLI-170.patch
        5 kB
        Henri Yandell
      2. remove-messages-to-stderr.patch
        1 kB
        J. Lewis Muir

        Activity

        Hide
        ebourg Emmanuel Bourg added a comment -

        I agree the messages appearing on running the tests are misleading, however I don't think it's a good idea to remove them, it provides a useful information about the cause of the error. It's possible to remove the messages from the tests by redirecting the output to a file, I plan to change the pom to do this.

        Show
        ebourg Emmanuel Bourg added a comment - I agree the messages appearing on running the tests are misleading, however I don't think it's a good idea to remove them, it provides a useful information about the cause of the error. It's possible to remove the messages from the tests by redirecting the output to a file, I plan to change the pom to do this.
        Hide
        jlmuir J. Lewis Muir added a comment -

        Changing the Maven POM will only solve the problem in a superficial way, and only when the test cases are run using Maven. I use Eclipse and its built-in JUnit support to run the test cases. In this case, and very likely in the case of other IDEs, the POM file is not used, so the error messages will continue to be written to stderr whenever the unit tests are run.

        If you think the information provided by the messages to stderr are useful, I suggest changing the methods to throw exceptions for these error cases. But if you don't like that, I'd say the next best thing would be to write the messages to some logging framework so there's good control over what messages get written where. In this case, I think the default should be that the messages are not written, and the developer would need to explicitly turn them on.

        I don't think a library should ever write error messages to stderr on its own; that's poor design. When I use a library in software I write, I expect the library to never write to stdout nor stderr unless I have told it to (e.g. a logging framework library or a function specifically for writing to those streams). I want to have complete control over what gets written to stdout and stderr.

        Show
        jlmuir J. Lewis Muir added a comment - Changing the Maven POM will only solve the problem in a superficial way, and only when the test cases are run using Maven. I use Eclipse and its built-in JUnit support to run the test cases. In this case, and very likely in the case of other IDEs, the POM file is not used, so the error messages will continue to be written to stderr whenever the unit tests are run. If you think the information provided by the messages to stderr are useful, I suggest changing the methods to throw exceptions for these error cases. But if you don't like that, I'd say the next best thing would be to write the messages to some logging framework so there's good control over what messages get written where. In this case, I think the default should be that the messages are not written, and the developer would need to explicitly turn them on. I don't think a library should ever write error messages to stderr on its own; that's poor design. When I use a library in software I write, I expect the library to never write to stdout nor stderr unless I have told it to (e.g. a logging framework library or a function specifically for writing to those streams). I want to have complete control over what gets written to stdout and stderr.
        Hide
        bayard Henri Yandell added a comment -

        Agreed. I've attached a backwards compat patch that means people wanting to turn off the System.err behaviour need to switch to calling getParsedOptionValue from the now deprecated getOptionObject.

        Show
        bayard Henri Yandell added a comment - Agreed. I've attached a backwards compat patch that means people wanting to turn off the System.err behaviour need to switch to calling getParsedOptionValue from the now deprecated getOptionObject.
        Hide
        bayard Henri Yandell added a comment -

        svn ci -m "Applying my patch from CLI-170 - TypeHandler prints messages to stderr. It doesn't change the default behaviour, but it does provide a new method which maybe called to not get the stderr output and instead get a checked exception thrown. "

        Sending src/java/org/apache/commons/cli/CommandLine.java
        Sending src/java/org/apache/commons/cli/TypeHandler.java
        Transmitting file data ..
        Committed revision 735247.

        Show
        bayard Henri Yandell added a comment - svn ci -m "Applying my patch from CLI-170 - TypeHandler prints messages to stderr. It doesn't change the default behaviour, but it does provide a new method which maybe called to not get the stderr output and instead get a checked exception thrown. " Sending src/java/org/apache/commons/cli/CommandLine.java Sending src/java/org/apache/commons/cli/TypeHandler.java Transmitting file data .. Committed revision 735247.

          People

          • Assignee:
            Unassigned
            Reporter:
            jlmuir J. Lewis Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development