sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request: Sqoop2: Add cloning ability to model classes
Date Thu, 02 May 2013 00:19:11 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10886/#review20038
-----------------------------------------------------------


Hi Vasanth,
thank you very much for working on this!

I was thinking about the cloneForms() methods and it's usage of "instanceof" operator. What
about creating a clone method on each model object and then just call that method instead
of quite long if-elif*-else statement? I think that this way we would be able to overcome
the need for instanceof. We can make the method parametric to see if user also wants to copy
over the values or not.

Jarcec

- Jarek Cecho


On May 1, 2013, 9:01 p.m., vasanthkumar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10886/
> -----------------------------------------------------------
> 
> (Updated May 1, 2013, 9:01 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Description
> -------
> 
> Cloning class by copy constructor
> 
> 
> This addresses bug SQOOP-995.
>     https://issues.apache.org/jira/browse/SQOOP-995
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/model/FormUtils.java 80b10c8 
>   common/src/main/java/org/apache/sqoop/model/MConnection.java c31eafd 
>   common/src/main/java/org/apache/sqoop/model/MConnector.java 9207c62 
>   common/src/main/java/org/apache/sqoop/model/MFramework.java f5252ef 
>   common/src/main/java/org/apache/sqoop/model/MJob.java 5b50bfd 
>   common/src/test/java/org/apache/sqoop/model/TestMConnection.java c3469ff 
>   common/src/test/java/org/apache/sqoop/model/TestMConnector.java 716b124 
>   common/src/test/java/org/apache/sqoop/model/TestMFramework.java a5366ca 
>   common/src/test/java/org/apache/sqoop/model/TestMJob.java dbc791e 
> 
> Diff: https://reviews.apache.org/r/10886/diff/
> 
> 
> Testing
> -------
> 
> Done
> 
> 
> Thanks,
> 
> vasanthkumar
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message