Refactoring a long method that just fills

Question

I am refactoring a method that is over 500 lines (don't ask me why)

The method basically requests a list of maps from the database and for each map in the list performs some calculations and adds the value of this calculation to the map. However, it is too much calculation and it becomes that the code has already reached more than 500 lines!

Here is a preview example:

public List<Hashmap> getProductData(...) {
   List<Hashmap> products = productsDao.getProductData(...);
   for (Product product: products) {
       product.put("Volume",new BigDecimanl(product.get("Height")*
              product.get("Width")*product.get("Length"));
   //over 10 more lines like the one above
      if (some condition here) {
         //20 lines worth of product.put(..,..) 
      } else {
         //20 lines worth of product.put(..,..)
      }
      //3 more if-else statements like the one above
      try {
         product.put(..,..)
      } catch (Exception e) {
         product.put("",..)
      }
      //over 8 more try-catches of the form above
   }

Any ideas on how to do this?

Answer 1

The simple idea that comes to my mind for dividing the method is to "find smaller tasks with feeling", so here are some tips:

Make method for item level

processCollection(collection) {
// startup code
for (Item i: collection)
  processCollectionItem(i, ...other args...);
// final code
}

// does a
instr 1
instr 2
instr 3
instr 4
// does b
instr 5
instr 6
instr 7
instr 8

a(...);
b(...);

, -

map.put(a[0], b[0]);
map.put(a[1], b[1]);
...

putKeysAndValues(a, b);

putKeysAndValues(a, b) {
  for (int i=0; i<a.length; i++)
     map.put(a[i], b[i]);
}

Answer 2

-, :

for (Product product: products) {
   handleProduct(product);
}


private void handleProduct(Product product) throws Exception {
  if (condition1) {
     handleProduct1(product);
  }
}

, - .

Answer 3

, , , :

• , Product, Map (, ). , ; , .
• , , 500 . , .

/**
 * Our visitor definition, responsible for processing each product.
 */
public interface ProductVisitor {
  void processProductA(ConcreteProductA a);

  void processProductB(ConcreteProductB b);
}

/**
 * Top-level product definition.
 */
public interface Product {
  void process(ProductVisitor v);
}

/**
 * A conrete product implementation.  Makes a callback to the visitor
 * allowing itself to be processed.
 */
public class ConcreteProductA implements Product {
  public void process(ProductVisitor v) {
    v.processProductA(this);
  }
}

/**
 * As per the previous product implementation but makes a *different*
 * callback to the visitor.
 */
public class ConcreteProductB implements Product {
  public void process(ProductVisitor v) {
    v.processProductB(this);
  }
}

, if-then , . , .

Answer 4

, dao... , :

public List<Hashmap> getProductData(...) {
   List<Hashmap> products = productsDao.getProductData(...);
   try{
     addVolume(products);
     ...
     doCondition1(products);
     ...
   } catch (E1 e){
     ...
   } ... {
   }
}

public addVolume(List<HashMap> products) throws E1, E2, ... {
  for(HashMap product : products){
    product.put("Volume",new BigDecimanl(product.get("Height")*
              product.get("Width")*product.get("Length"));
  }
}

... do other things ...

public doCondition1(List<HashMap> products) throws E1, E2, ...{
  for(HashMap product : products){
    if(condition1){
      ...
    }else{
      ...
    }
  }
}

...condition 2, condition 3, ...

,

getProductData (...)

Answer 5

you can shorten a method by moving conditional puts into a separate method and calling it from the current method.

eg.

...
if (some condition here) 
 { method1(map);} 
  else 
{method2(map);}

Also, if there are general tasks, pull them in separate methods.