0

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 
    }

}
5
  • 1
    What does this have to do with JNI (tagged as java-native-interface)? Did you even read the description of the tag when you selected it? Commented Sep 21, 2020 at 21:40
  • 2
    As Charlie put below, you can use the instanceof keyword to identify the object type. However, your structuring looks like it'd benefit from keeping the current setup but maybe adjusting the method names to be the same so it's overloaded and better follows the "tell, don't ask" principle. Commented Sep 21, 2020 at 21:46
  • You asked me below: „how do you know what are you going to retrieve at a given index?“. To answer that question satisfactorily would require you to clarify your question with an answer to: «how would a caller know at what given index an entity is saved?». Currently the code in your question inserts entities randomly. In your examples you don't make it clear how a caller of ItemsInMemoryDao.save() would know at what index an entity is saved. I assume you're considering editing your question to specify that an acceptable answer must support index-based lookup? Correct? TIA. Commented Sep 22, 2020 at 15:49
  • You're right, I didn't clarify that enough. I will reply in the comments for your answer. Commented Sep 22, 2020 at 20:51
  • …I didn't clarify that enough…“ – @JanHorčička — Not only did you not clarify it, you didn't even mention it at all in your original question. Nor 24+ hours after you were requested to. No skin off my teeth though. I'm just sayin'; as an offering of advice on how to write better questions in the future. Commented Sep 23, 2020 at 18:19

4 Answers 4

2

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.

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

Comments

1

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.

Comments

1

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>{

}

Comments

0

…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.

7 Comments

Does this save all of the objects into one list or are there 3 lists? If only one list, how do you know what are you going to retrieve at a given index?
…Does this save all of the objects into one list or are there 3 lists?…“ – @JanHorčička — Thanks for the heads-up. I just reread the Requested state part of your question more carefully. I read it too fast the first time around and didn't catch that part at first. Let me know if none of the other answers meet your needs? If none do, then I will refactor the demo and edit my answer accordingly to address retrieval. In the meantime, please let me know: Is the demo link broken? TIA.
The best other answer is the one with method overloading. Your solution would be even better as long as it saves objects into three different lists, which I understand it doesn't? The demo link works.
…saves objects into three different lists…“ – @JanHorčička — To paraphrase your objection to duplicate methods: «I have to repeat the [List declaration] 3 times, once for each type, although in each case, the [List declaration] does exactly the same thing». IMHO, if you're genuinely committed to reducing code duplication, then the redundant List declarations present another opportunity to dedupe even more. IMHO, using one List that effectively does the exact same thing as three, makes your code 3X simpler. BTW I replaced the demo link with a new one. Does it work?
…how do you know what are you going to retrieve at a given index?…“ – @JanHorčička — Is leaking to consumers, the implementation detail that your entities are stored in an indexed data structure, all that necessary IRL? In my experience with data access-related API's it's never mattered what data structure the „DB“ used under the hood. IRL, persisted entities are ordinarily referenced by a unique ID; not by an index in some kind of array. What if you discover a better data structure down the road? One that's not indexed-based. You'd break your consumers if you stopped using indices.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.