[ https://issues.apache.org/activemq/browse/CAMEL-398?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=48971#action_48971 ]
Claus Ibsen commented on CAMEL-398: ----------------------------------- v0.97 review ========== BindyCsvFactory - why not use a boolean for skipFirstLine instead of string. Its confusing to use y or n when its really a boolean - getCsvRecordParameters() does no return anything. Please rename it to initCsvRecordParameters() - all method is public. please use protected / private for methods that should not be exposed - mapLinkField is never used (= no read from it, only put into it) - findXX methods does not return anything. Consider renaming them to initXXX instead - you are using this for empty test: data.get(pos).equals("") what if the string is an space? " " well that is hard to tell I guess if it should be consider empty or not? - Ah we use field access for setting values. We should use setXXX instead. Dont worry Camel has such support org.apache.camel.util.IntrospectionSupport.setProperties You should use this class instead (we do this in Camel internally as well). - This code is hard to read: format = FormatFactory.getFormat(field.getType(), dataField.pattern() != null ? dataField.pattern() : null, dataField.precision()); Consider getting the Pattern as a local variable first and call it as format = FormatFactory.getFormat(field.getType(), pattern, dataField.precision()); - You are doing the notNull test in the while loop. You should do this outside (before) ObjectHelper.notNull(this.separator, "The separator has not been instantiated or property not defined in the @CsvRecord annotation"); - This method throws 3 exceptions: - When creating new instances of a class you can use org.apache.camel.util.ObjectHelper as it will handle all the nasty exception for you (we do that internally in Camel) - You are using 999 as default for precision. Why not use 0? - ObjectHelper.notNull(this.pattern, "No pattern defined."); Just set the text as "pattern" as the notNull method will add a text description that pattern is null. => otNull(this.pattern, "pattern"); Other - getDateFormat() you can just do: return new SimpleDateFormat(pattern); - dont javadoc methods that is already javadoced at the interface (such as all the format and parse methods in all the xxxFormat classes) - The class javadoc for BindyCsvDataFormat says its about XML ??? - If you see the code from IDEA there are many hints for the code that can be fixed (doesnt Eclipse help you there? code that is not used, auto boxing, etc. etc.) - use string equals for string comparision. you can not use == or != in java for strings. (result.get(firstPosition) != "") > Map the content of a CSV file to a POJO using @annotation and .convertBodyTo() > ------------------------------------------------------------------------------ > > Key: CAMEL-398 > URL: https://issues.apache.org/activemq/browse/CAMEL-398 > Project: Apache Camel > Issue Type: New Feature > Components: camel-core > Affects Versions: 1.4.0 > Reporter: Charles Moulliard > Assignee: Claus Ibsen > Fix For: 2.0.0 > > Attachments: camel-bindy-v0.95.zip, camel-bindy-v0.96.zip, > camel-bindy-v0.97.zip, camel-bindy.zip > > > Hi, > It should be nice if in a next relase of Camel, it will be possible to map > the content of a CSV file to a POJO using @annotation. > For the moment, I use an ArrayList + iterator (see code hereafter) to achieve > the extraction of the content but I'm sure that we can simplify this code > using @Annotation > and the following action (.convertBodyTo(Order<List>) by example. > Current situation > Camel route > from("file:///c:/temp/test?noop=true") > .unmarshal().csv() > .to("bean:converter?methodName=TransformMessage"); --> should be replaced by > something like .convertBodyTo(Order<List>) > Converter class > public void TransformMessage(Exchange in) { > process(in.getIn().getBody(List.class)); > } > @SuppressWarnings("unchecked") > private void process(List messages) { > > // Iterate through the list of messages > for (Iterator<ArrayList> it = messages.iterator(); > it.hasNext();) { > // Split the content of the message into field > message = it.next(); > field = (String[]) message.toArray(); > order = new Order(); > order.setId(Integer.valueOf(field[0]).intValue()); > order.setBank(field[1]); > order.setAmountFrom(Double.parseDouble(field[2])); > order.setAmountTo(Double.parseDouble(field[3])); > order.setOrderInstruction(field[4].trim()); > this.orderService.createOrder(order); > } > } > Regards, > Charles -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.