Java code refactoring issue

Question

I have a similary class structure as below:

       public class Foo extends Base{
                ...
        }   

        public class Bar extends Base{
                ...
        }

        public class Aos extends Base{
                ...
        }

        public class Wrap{
            List<Foo> getFooList();
            List<Bar> getBarList();
            List<Aos> getAosList();
        }

Fold fooFold = getFooFold();
Fold barFold = getBarFold();
Fold aosFold = getAosFold();

    // Code to refactor
        for (Obj e : fooFold.getObj()) {
               Foo foo = new Foo(.., .., ..);
               for (Som s: e.getSom())) {
                    doSom();
               }
               wrap.getFooList().add(foo);
        }

        for (Obj e : barFold.getObj()) {
               Bar bar = new Bar(.., .., ..);
               for (Som c : e.getSom())) {
                    doSom();
               }
            wrap.getBarList().add(bar);
        }

        for (Obj e : aosFold.getObj()) {
             Aos aos = new Aos(.., .., ..);
             for (Som c : e.getSom())) {
                    doSom();
            }
               wrap.getAosList().add(aos);
        }

How can i refactor the for-loops? (The for-loops are a "little" more complex. The logic do always the same. The iteration over the list, the creation of the object and adding the object to the list are different. )

are getFooSpecObj(), getAosSpecObj(), and getBarSpecObj() have to be called seperately? In that case, you have to have 3 for- loops. Apart from that extract the nested for into a private function — Chip
– Chip, Commented Oct 23, 2013 at 8:38
The calls to doSom() in the loops - as far as I can see, they don't use the current iteration of Som, so what are they for? If it was c.doSom() or doSom(c) it would make more sense. Also, I hope those aren't your real variable names. Variable names should be descriptive. — NickJ
– NickJ, Commented Oct 23, 2013 at 8:49

Chip · Accepted Answer · 2013-10-23 08:41:12Z

1

If all the functions are separate, the only thing you can refactor is put the common code in a private function

private doCommonThings(Base e) {
      for (Som c : e.getSom())) {
            doSom();
       }
}

then use it in all your loops

for (Obj e : getFooSpecObj()) {
       Foo foo = new Foo(.., .., ..);
       doCommonThings(e);
       wrap.getFooList().add(foo);
}

answered Oct 23, 2013 at 8:41

Chip

3,32625 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Paul Wagland · Accepted Answer · 2013-10-23 08:38:02Z

0

If your code is really as simple as you have it above, you will probably find that any refactoring that you do to simplify it, will only end up making it more complex.

answered Oct 23, 2013 at 8:38

Paul Wagland

29.4k10 gold badges55 silver badges75 bronze badges

Comments

Yuli Reiri · Accepted Answer · 2013-10-23 11:48:10Z

Here is hard to judge about re-factoring direction as some parts of implementation are missing. Here is my guess how to proceed in this case:

public class ListBase< T >  {

    private List< T > _list;

    public ListBase(ListFactory< List < T > > list_factory ){
       _list = ( List< T > ) list_factory.create ( );
    }

    public void foreach ( CallbackInterface< T > callback ){
       for ( T i :getList( ) ){
           callback.process ( i );
       }
    }

    public List < T > getList ( ){
       return _list;
    }
}

public interface ListFactory< T >  {
    List< T > create ();
}

public interface CallbackInterface < I > {
    void process ( I element );
}

class ListFactorryArrayList < T > implements ListFactory < T > {
    @Override
    public ArrayList< T > create( ) {
        return new ArrayList < T > ( );
    }
}
public class Wrap {

    public ListBase< Foo > _listFoo = new ListBase<Foo >( new ListFactorryArrayList < List<Foo> > () );
    public ListBase< Bar > _listBar = new ListBase<Bar >( new ListFactorryArrayList < List<Bar> > () );
}

public class Main {

    public static void main(String[] args) {
    Wrap w = new Wrap ();
        w._listFoo.getList().add( new Foo () );
        w._listFoo.foreach( new CallbackInterface <Foo > () {

            @Override
            public void process(Foo element) {
                // do smth with foo object
            }
        });
    }
}

Collectives™ on Stack Overflow

Java code refactoring issue

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related