Menu ▾ ▴

#132 populating Bean member variables from factory methods?

closed
None
5
2020-10-25
2020-05-31
Andrew M
No

With a csv like this:

country name, currency
Hong Kong, HKD
USA, USD
Canada, CAD

I need to populate classes based on some static method roughly like this:

class Country {
  @CsvBindByName(column = "country name") 
  String name;

  @CsvBindByName(column = "currency") 
  @SomeAssignmentProcessor(method = java.util.Currency::getInstance) 
  java.util.Currency ccy;
}

Is that possible now?

Discussion

1 2 > >> (Page 1 of 2)
  • Andrew Rucker Jones

    Well, you could always write a small custom converter for that purpose.

    Alternately, you could define a String field for that column, map that, but in the setter for that field, set your Currency field instead.

     
    • Andrew M

      Andrew M - 2020-06-12

      Would you consider adding that Converter to the codebase? I attached the one I am using.

       
      • Andrew Rucker Jones

        Looks nice. Any chance you could contribute tests also?

         
        • Andrew M

          Andrew M - 2020-06-15

          Sure. What class(es) would you suggest I add these tests to? StatefulBeanToCsvWithCSVWriterTest.?

           
          • Andrew Rucker Jones

            We already have one custom coonverter. I suggest you look at how that is tested and see if that makes sense for your converter also.

             
            • Andrew M

              Andrew M - 2020-06-15

              You're talking about ConvertGermanToBoolean?

               
              • Andrew Rucker Jones

                Yes.

                 
  • Andrew Rucker Jones

    I am thinking I would like native support for Currency. It looks quick and easy—it would be your code in a different class and a couple lines to link it into … the mapping strategy, I think. You interested in doing it that way? It would be com.opencsv.bean.ConverterCurrency, analog to ConverterDate, ConverterEnum, ConverterNumber, and ConverterPrimitiveTypes.

     
    • Andrew M

      Andrew M - 2020-07-01

      Sounds great. I can create that class.

       
      • Andrew M

        Andrew M - 2020-07-01

        Does this look ok? Can we specify the String.class in the ConverterCurrency constructor the way I've done? Working on tests....

         

        Last edit: Andrew M 2020-07-01
        • Andrew M

          Andrew M - 2020-07-02

          I added tests and a 51st field to AnnotatedMockBeanFull

           /**
          
               * Field for annotation tests.
               * <p>Used for the following test cases, reading:</p>
               * <ul>
               * <li>{@link com.opencsv.bean.AnnotationTest#testGoodDataByName()}</li>
               * </ul>
               */
              @CsvBindByName(column = "currency1", required = false)
              @CsvBindByPosition(position = 51, required = false)
              private Currency testCurrency;
          

          However it seems like with that extra field I have to add a 51st field to almost every row of every CSV in in the test resources.

           
          • Andrew Rucker Jones

            Yeah, the test thing is kind of a pain, I admit.

             
        • Andrew Rucker Jones

          Is there any advantage to having an annotation CsvCurrency? It carries no information. I would simply check the type of the field involved, and if it is Currency, use the new converter.

          Your parameters to the new converter are sparse. Take a look at the parameters passed to the other converters and how they are used. I would expect all existing functionality to be supported, and that includes things like format strings. Looking at the other converters should also answer your question about passing in String.class as the type.

           
          • Andrew M

            Andrew M - 2020-07-02

            If we can automatically apply the converter without an annotation whenever the field type is Currency then that makes the most sense.

            I believe there should be no parameters since the currency code is not language or locale specific. Is that right?

             
            • Andrew M

              Andrew M - 2020-07-02

              Does this look ok?

              I changed testUnknownElementType from Currency to TimeZone.

              Why is there both opencsv.properties and opencsv_en.properties? I'm guessing the first one becomes the default when the locale is not en, fr, de or pt_br? Could one be removed?

               
              • Andrew Rucker Jones

                Getting closer.

                Yes, both properties files are necessary. Bug 180 for reference.

                In ConverterCurrency:

                1. You need to pass Currency.class to the base consrtuctor, especially since you then use "type" and "type.getName()" for at least one error message (as you should).
                2. You need to add an initCause() to the first exception you throw.
                3. You need to internationalize (and localize as much as possible) the error message in the second exception you throw.
                4. In the second exception you should be using the constructor with three parameters (CsvDataTypeMismatchException).

                And then of course tests. :)

                 
                • Andrew M

                  Andrew M - 2020-07-04

                  Attached. I'll add a couple tests too

                   
                  • Andrew M

                    Andrew M - 2020-07-04

                    Where would you want those tests added? I presume we need to test:

                    • parsing the mocked bean to CSV
                    • parsing CSV to bean
                    • getting exception when passed bad currency value
                    • getting exception when passed non-Currency object

                    Would you add a field to MockBean for this?

                    public class MockBean {
                       private String name;
                       private String id;
                       private String orderNumber;
                       private int num;
                       private double doubleNum;
                       private Currency ccy;
                    
                     
                    • Andrew Rucker Jones

                      The code looks good.
                      In your tests under "bad" values don't forget null and empty values, as applicable. That's bitten me a time or two.
                      I would not expect this to be a field in MockBean, because MockBean is not for annotation tests. I would expect to see it in AnnotatedMockBeanFull, though I am open to other suggestions. When I implemented the Java 8 Time API, I put all of those tests in their own mocks. But that was also a lot of fields and a lot of tests.

                       
                      • Andrew M

                        Andrew M - 2020-07-13

                        Are you suggesting I add a 51st field to AnnotatedMockBeanFull?

                        @CsvBindByName(column = "currency1", required = false)
                        @CsvBindByPosition(position = 51, required = false)
                        private Currency testCurrency;
                        
                         
                        • Andrew Rucker Jones

                          Yes.

                           
  • Andrew Rucker Jones

    Haven't heard back on this in a while. I like the idea of including this in the next release. Would you be willing to give me what you have with a summary of what's incomplete so I can finish it?

     
  • Andrew Rucker Jones

    • status: open --> pending
    • assigned_to: Andrew Rucker Jones
     
  • Andrew Rucker Jones

    I finished the tests and merged this into master. It will go out with the next release. Thanks for your contribution!

     
  • Andrew Rucker Jones

    • status: pending --> closed
     
1 2 > >> (Page 1 of 2)

Log in to post a comment.