Jan Horčička
Jan Horčička

Reputation: 853

Add object to list based on the object type

I have a class that stores objects in lists. I have 3 different types of lists and I want to save the object in their respective list. As you can see, I have to repeat the method 3 times, once for each type, although in each case, the method does exactly the same thing.

Question:

Is there a way to write the same functionality with just one method using for example generics or interface?

Original code:

@Repository
public class ItemsInMemoryDao {
    
    static List<MyCompany> companies = new ArrayList<>();
    static List<Financial> financials = new ArrayList<>();
    static List<Stock> stocks = new ArrayList<>();;

    // TODO: Rewrite using generics or interface?
    static void saveCompany(MyCompany company) {
        companies.add(company);
    }

    static void saveFinancial(Financial financial) {
        financials.add(financial);
    }

    static void saveStock(Stock stock) {
        stocks.add(stock);
    }

}

Requested state:

@Repository
public class ItemsInMemoryDao {

    static List<MyCompany> companies = new ArrayList<>();
    static List<Financial> financials = new ArrayList<>();
    static List<Stock> stocks = new ArrayList<>();;

    static void save(Object object) {
        // implementation here 
    }

}

Upvotes: 0

Views: 1013

Answers (4)

martinspielmann
martinspielmann

Reputation: 576

Don't repeat yourself is often a good idea. But, although this one method is the same for all items, I suggest to create a seperate DAO class for each item type. When future methods are added, such mixed classes will likely become bloated.

However, to not repeat yourself in the save method, you could use an abstract super class and generics as you suggested already:

public abstract class InMemoryRepo<T> {
    
    // note: 'static' is typically not needed here.
    // @Repository indicates that your DAO will be created and managed as a signelton bean right?
    private List<T> items = new ArrayList<>();

    public void save(T item){
        items.add(item);
    }
}

@Repository
public class CompanyInMemoryDao extends InMemoryRepo<MyCompany>{

}

@Repository
public class FinancialInMemoryDao extends InMemoryRepo<Financial>{

}

@Repository
public class StockInMemoryDao extends InMemoryRepo<Stock>{

}

Upvotes: 1

deduper
deduper

Reputation: 1964

…Is there a way to write the same functionality with just one method using for example generics or interface?…

The simplest way to do what you require is the latter…

public interface Saveable { 

    long getId( );

    void setId( long id );
}

Then to further reduce duplication of code…

public abstract class AbstractSaveable implements Saveable {
    
    protected long id;
    
    @Override
    public long getId( ){ return this.id; }
    
    @Override
    public void setId( long id ){ this.id = id; }
}

Your entities would extend that…

public class MyCompany extends AbstractSaveable { … }

public class Financial extends AbstractSaveable { … }

public class Stock extends AbstractSaveable { … }

So the only generics you really, truly need in such a simple use-case are the built-in ones…

…
public class ItemsInMemoryDao { 

    static private List< Saveable > db = new ArrayList< >( );    
    
    public void save( Saveable ntt ) {
        db.add( ntt ); 
    }

    public Optional< Saveable > findById( final long id ) { 
        return db.stream( ).filter( ntt -> ntt.getId( ) == id ).findFirst( );
    }
}

I've confirmed that this works in this experiment

…    
ItemsInMemoryDao dao = new ItemsInMemoryDao( );
    
dao.save( new MyCompany( 1 ) );

dao.save( new Financial( 2 ) );

dao.save( new Stock( 3 ) );
…

Of course, you could always over-engineer it use generics…

public class OverEngineeredDao< NTT extends Saveable > { 
    
    private List< NTT > db = new ArrayList< >( );   
    
    public void save( NTT ntt ){
        
        db.add( ntt );
    }

    public Optional< NTT > findById( final long id ) { 
        return db.stream( ).filter( ntt -> ntt.getId( ) == id ).findFirst( );
    }
}

…To get the same result…

…
OverEngineeredDao< Saveable > oeDao = new OverEngineeredDao< >( );
    
oeDao.save( new MyCompany( 1 ) );

oeDao.save( new Financial( 2 ) );

oeDao.save( new Stock( 3 ) );
…

Which I print out to confirm it works…

                                 Optional[MyCompany [ id: 1 ]]
                                 Optional[Financial [ id: 2 ]]
                                     Optional[Stock [ id: 3 ]]
                                 Optional[MyCompany [ id: 1 ]]
                                 Optional[Financial [ id: 2 ]]
                                     Optional[Stock [ id: 3 ]]
                                         EXPERIMENT SUCCESSFUL

You can see both approaches successfully running here.

Upvotes: 0

Andreas
Andreas

Reputation: 159106

Is there a way to write the same functionality with just one method using for example generics or interface?

Yes, and no but yes but don't do it.

Can it be done with "one" method called save?
Yes, using method overloading.
This is the recommended approach.

static void save(MyCompany company) {
    companies.add(company);
}

static void save(Financial financial) {
    financials.add(financial);
}

static void save(Stock stock) {
    stocks.add(stock);
}

Can it be done with truly one method?
Yes, using instanceof in the implementation.
This is highly discouraged, since you will lose the type-safety of Java.

static void save(Object object) {
    if (object instanceof MyCompany)
        companies.add((MyCompany) company);
    else if (object instanceof Financial)
        financials.add((Financial) company);
    else if (object instanceof Stock)
        stocks.add((Stock) company);
    else
        throw new IllegalArgumentException("Argument is of unknown type: " + object.getClass().getName());
}

E.g. if a caller tries to call save("Foo"), the first solution will fail to compile, so you instantly knows something it wrong, while the second solution will compile just fine, and you don't know something is wrong until you try running the code in question.

Upvotes: 1

Charlie Armstrong
Charlie Armstrong

Reputation: 2342

Any reason you couldn't just use instanceof? Here's one possible implementation:

static void save(Object object) {
    if (object instanceof MyCompany) {
        companies.add((MyCompany) object);
    } else if (object instanceof Financial) {
        financials.add((Financial) object);
    } else if (object instanceof Stock) {
        stocks.add((Stock) object);
    } else {
        throw new IllegalArgumentException();
    }
}

That said, I can't say I really like this implementation. The chained else/ifs just look unnecessarily messy to me. Is there any reason you need to consolidate everything into one method? If it was me, and there was no reason to do otherwise, I would just leave it the way you had it.

Upvotes: 2

Related Questions