small patch

[sm_zl_kimi]

a small patch.just replace the following function in CSV.java    

 

     private void setPlanting(CPSPlanting plant,final int INDEX,String value)
     {
         plant.set(INDEX,value);
     }

 

 

     private void setCropsAndVarieties(CPSCrop crop,final int INDEX,String value)
     {
         crop.set(INDEX,value);
     }

 

by the way, any other work I can do for this project?

[Clayton Carter]

Great! I'm still waiting on a chance to test this out.

Yes, there is plenty more to do. I've tried to fill out more issues
in the projects issue tracker:

http://code.google.com/p/cropplanning/issues/list

Are you comfortable with SQL? There are a couple of SQL issues that
need some help.

Clayton

[Clayton Carter]

It looks like you also created an interface CPSImporter. Can you send
me that file, too?

C

[sm_zl_kimi]

I think sql work is ok,I have  experience  of mysql and sqlsever,however I know little about hsql.

 

2008/4/4, Clayton Carter <croppl...@gmail.com>:

[sm_zl_kimi]

sorry I forget : )

 

 

2008/4/4, Clayton Carter <croppl...@gmail.com>:

[sm_zl_kimi]

CPSImporter.java copied from your CPSExporter.java

as my careless,I forget to replace "exporter" with "importer" in comments.I found out just now : (

 

try:

 

package CPS.Module;

import CPS.Data.CPSCrop;
import CPS.Data.CPSPlanting;
import java.util.ArrayList;
/**
 * A simple interface for classes that provide the capabilty to import
 * data to the file system.
 */
public interface CPSImporter {

    public ArrayList<CPSCrop>  importCropsAndVarieties( String filename );
                                         
    public ArrayList<CPSPlanting>  importCropPlan( String filename );

    public String getImportFileDefaultExtension();
   
}

[Clayton Carter]

No problem. I just got around to testing this (I had to add some
"Import" code to the gui) and I ran into a couple of things:

1. Importing crop and varieties worked great; good work!
2. Plantings not so much. How did you go about testing this? It
looks like the code for importing is running OK, but I get lots of
errors when I take the list of imported plantings and try to create
each of them in the DB. Did you get that to work? It looks like
there's a combination of bad design (on my part) and insufficitent
type checking (on your part). It looks like you're saving everything
to the planting objects as Strings (insufficient type checking) and
the abstract type/prototype system I was using isn't catching this
(bad design).

I would suggest you alter the setPlanting() method w/ a CASE statement
that groups INDEXes by type (booleans and dates at least) and then
does some further processing of these so that the value that's saved
using the set(...) method is correct for that property number. For
example:

switch ( INDEX ) {

case PROP_DATE_PLANT:
case PROP_DATE_TP:
case ....:

// parse the date into a Date object and then pass the Date object to set()

case PROP_DONE_PLANT:
case PROP_DONE_TP:
case DIRECT_SEED:

// parse the boolean value into a Boolean object ....

Shouldn't be a huge pain.

3. When you catch Exceptions or print error message, please make you
error messages a bit clearer than simply "wrong". A very simple way
is to just catch the exception and print a stack trace (via the
printStackTrace() method); that at least gives us information about
what went wrong.

But, like I said, importing Crops and Vars worked perfectly. Thanks again!

Clayton

[Clayton Carter]

One more thing: to handle the String->Date parsing, there're some
static methods in the CPSDateValidator class. You can also just
instantiate that class and rely on the multi-format parsing that it
can do.

C

[sm_zl_kimi]

I have a little suggestion

you suggest to write like

switch ( INDEX ) {
 case PROP_DATE_PLANT:
 case PROP_DATE_TP:
 case ....:

 

however ,setPlanting() I wrote at the first contains a SWITCH too, and call setXXX() in CPSPlanting,not set() in CPSRecord,

so I think setPlanting I wrote before wouldn't have any type problem.

but as I use switch in a function,it seems too long,ugly. so I suggest  overload setDatum in Datum,like

setDatum(String)  then convert String to type it should be and do some type checking.

what do you think about it?

 

2008/4/4, Clayton Carter <croppl...@gmail.com>:

[Clayton Carter]

I'm not sure that that's the direction in which I want to go.
CPSDatum is implemented w/ generics and I'm not really sure it's
possible to do specific things based on what type is specified. Does
that sound correct? Moreover, we would have to create code to
anticipate and handle all possibilities. Not that this is a BAD
thing, but it seems to be the wrong direction.

My suggestion was not a giant switch statement like how you first
implemented the code. Rather, I was just suggesting that you use the
switch to catch the properties whose types you know. This is what I
meant:

private void setPlanting(CPSPlanting plant,final int INDEX,String value)
{

switch ( INDEX ) {
case CPSDataModelConstants.PROP_DATE_PLANT:
case CPSDataModelConstants.PROP_DATE_TP:
case CPSDataModelConstants.PROP_DATE_HARVEST:
plant.set( INDEX, CPSDateValidator.simpleParse( value ) );
return;

case CPSDataModelConstants.PROP_DONE_PLANTING:
case CPSDataModelConstants.PROP_DONE_TP:
case CPSDataModelConstants.PROP_DONE_HARVEST:
case CPSDataModelConstants.PROP_DIRECT_SEED:
case CPSDataModelConstants.PROP_FROST_HARDY:
plant.set( INDEX, new Boolean( value ));
return;

default:
plant.set(INDEX,value);
return;
}

}

I made the change. Please let me know what you think.

Clayton

[sm_zl_kimi]

private void setPlanting(CPSPlanting plant,final int INDEX,String value)
{
      switch ( INDEX ) {
          case CPSDataModelConstants.PROP_DATE_PLANT:
          case CPSDataModelConstants.PROP_DATE_TP:
          case CPSDataModelConstants.PROP_DATE_HARVEST:
            plant.set( INDEX, CPSDateValidator.simpleParse( value ) );
            return;

         case CPSDataModelConstants.PROP_DONE_PLANTING:
         case CPSDataModelConstants.PROP_DONE_TP:
         case CPSDataModelConstants.PROP_DONE_HARVEST:
         case CPSDataModelConstants.PROP_DIRECT_SEED:
         case CPSDataModelConstants.PROP_FROST_HARDY:
            plant.set( INDEX, new Boolean( value ));
            return;
       

         case CPSDataModelConstants.PROP_PLANTING_ID:
         case CPSDataModelConstants.PROP_MATURITY:
         case CPSDataModelConstants.PROP_TIME_TO_TP:
         case CPSDataModelConstants.PROP_ROWS_P_BED:
         case CPSDataModelConstants.PROP_YIELD_NUM_WEEKS:
         case CPSDataModelConstants.PROP_MAT_ADJUST:
         case CPSDataModelConstants.PROP_PLANTING_ADJUST:
         case CPSDataModelConstants.PROP_MISC_ADJUST:
         case CPSDataModelConstants.PROP_PLANTS_NEEDED:
         case CPSDataModelConstants.PROP_PLANTS_START:
         case CPSDataModelConstants.PROP_ROWFT_PLANT:
         case CPSDataModelConstants.PROP_SPACE_BETROW:
         case  CPSDataModelConstants.PROP_SPACE_INROW:
            if (value.equals(""))
                plant.set(INDEX, new Integer(-1));
            else
             plant.set(INDEX, new Integer(value));
             return;
        
         case CPSDataModelConstants.PROP_CROP_NAME:
         case CPSDataModelConstants.PROP_VAR_NAME:
         case CPSDataModelConstants.PROP_KEYWORDS:
         case CPSDataModelConstants.PROP_OTHER_REQ:
         case CPSDataModelConstants.PROP_NOTES:
         case CPSDataModelConstants.PROP_STATUS:
         case CPSDataModelConstants.PROP_FLAT_SIZE:
         case CPSDataModelConstants.PROP_PLANTER:
         case CPSDataModelConstants.PROP_PLANTER_SETTING:
         case CPSDataModelConstants.PROP_CROP_UNIT:
         case CPSDataModelConstants.PROP_GROUPS:
         case CPSDataModelConstants.PROP_LOCATION:
         case CPSDataModelConstants.PROP_CUSTOM1:
         case CPSDataModelConstants.PROP_CUSTOM2:
         case CPSDataModelConstants.PROP_CUSTOM3:
         case CPSDataModelConstants.PROP_CUSTOM4:
         case CPSDataModelConstants.PROP_CUSTOM5:
            if (value.equals(""))
                plant.set(INDEX, new String());
            else
             plant.set(INDEX, value);
             return;
          
         case CPSDataModelConstants.PROP_YIELD_P_FOOT:
         case CPSDataModelConstants.PROP_YIELD_P_WEEK:
         case CPSDataModelConstants.PROP_BEDS_PLANT:
         case CPSDataModelConstants.PROP_FLATS_NEEDED:
         case CPSDataModelConstants.PROP_TOTAL_YIELD:
         case CPSDataModelConstants.PROP_CROP_UNIT_VALUE:
            if (value.equals(""))
                plant.set(INDEX, new Float(-1.0));
            else            
            plant.set(INDEX,new Float(value));
            return;
         
         default:
            plant.set(INDEX,value);
            return;
   }
}

 

I test it and it can work, save plants into ArrayList

but sorry I do not test whether it can import into HSQL well.

I don't know how to use HSQL, I should read CPS-HSQLDB

 

[Clayton Carter]

Good that it runs. Don't worry about explicitly testing it. I can
handle that. (I've created some code to make importing easy -- right
from the GUI -- similar to how export is done.)

This debate about switches and such is interesting and I'm not sure
where to go with it. For instance, the way the code is written, we
don't technically NEED to have any of the "case"s for types other than
Date()s. Everything else seems to just work. (So we would only need
the "case" statements for Dates and all of the rest could be caught by
the "default".) So, the question is, do we rely on this and use
simpler, cleaner code, or should we do as you have done above and
prepare more complete and robust, though less clean and attractive,
code.

Any thoughts?

Clayton

[sm_zl_kimi]

I am glad it runs.
about the debate,
just date() need case,so I
think a switch is ok. simpler better. we can do refactoring when we
really need to.

[Clayton Carter]

Hi all,

I have putback sm_zl_kimi's code adding CSV import. I have done some
very quick testing and it appears to work. The only issues I noted
looked like they were more of an issue w/ export than with import.

If anyone has tried to checkout and use the code recently, they will
have gotten a ClassNotFound exception at runtime. This putback should
fix that.

Clayton

[sm_zl_kimi]

classnotfound?I will checkout and test.
sorry for not sending any code for this project last week. I was
preparing for my application

[Clayton Carter]

The ClassNotFound exception was unrelated to code you had submitted.
I had refactored the CSV module/package and putback only part of the
changes. (I left out the actual CSV module, hoping to do some more
testing.) Anyone who checked out code would run into an Exception. I
believe that this should fix it. If we test and find problems, we can
fix them later in another putback.

C