Java vs. OO Design

Question

I'm working on an inherited system which has some design issues which "OO Buzzwords" frown upon, and some I personally dislike.

It's a stock and sales handling program for a comic book store.

I have an Article class, which can be any item (Magic cards, toys), and a Publication class that inherits from Article, which represents books and magazines. A Publication has Authors and optionally issue number, while an Article doesn't.

There's an Article Editor, which is a GUI to create and modify articles. Since there's the possibility to load a Publication with an error, and not add a volume number, the interface to work with an article is:

Article a = EntityManager.loadArticle(articleId);
ArticleEditor editor = new ArticleEditor(a);
a = e.getValue();

to allow a to be changed into a Publication, should the need arise.

One of my peeves is that this could be handled more gracefully if it used references, or ar least so it appears to me. My current version uses wraps the last 2 lines' pattern in a static version, but it still looks ugly because it looks too state-dependent.

---

The second issue comes with Java's (and most "enterprise" languages') lack of multiple dispatch: the EntityManager has a save() method, overloaded for both Articles and Publications. This causes a huge issue if I say, for example:

Article a = EntityManager.loadArticle(articleId);
ArticleEditor editor = new ArticleEditor(a);
a = e.getValue();
EntityManager.save(a);

This is currently "solved" by having the ArticleEditor save changes (which seems wrong).

I'm sure there must be some way to adjust the design to eliminate these issues. (I don't mind a whole re-write, if need be.)

How can I adjust the design to eliminate these issues?

Edit: Publication has Authors, too, not only Numbers.

Answer 1

1. What's the problem? It seems fine to me.
2. Can't you simulate multiple dispatch in Java using reflection?

Answer 2

I don't the first question. For the 2nd one, it's true that multiple dispatch is desired sometimes, however probably not in your case, a simple if(article instanceof Publication) save((Publication)article) is just fine. You have only a few classes, don't try to overdesign.

Answer 3

I don't know how radically you want to refactor this system, but I'm suspicious of why you would have a separate Publication class just because it's an Article which has a publication number. Especially when you mention the need to change an Article to a Publication on occasion. You could allow Article to have a "publication number" property which can be null if the article isn't a publication. That takes away the need for an Article to change into a Publication (just set the property to a non-null value) and it makes your problem with EntityManager go away too.

Of course there may well be other reasons for having a separate Publication class, I'm just going on what I see here.

Answer 4

I guess you meant a = editor.getValue(); in your examples?

Issue 1 is not really an issue, unless you are saying that your Article and Publication are immutable classes? The editor is created on an Article instance and if it works with that object reference its contents will change (posibly change it after a confirm/rollback descision in the UI.)

From your description I assume that Article acts as a "template" for Publication, and that in your UI the action create a Publication is separate from edit an Article or a Publication. You could code the creation of new Publication's as follows:

Publication p = EntityManager.newPublication(articleId);
ArticleEditor editor = new ArticleEditor(p);

Issue 2 can be tackled by implementing a save method in Article and overloading it in Publication:

interface ArticleStore {
}

class Article {
    void save(ArticleStore store);
}

class EntityManager implements ArticleStore {
     void save(Article a) {
         a.save(this);
     }
}

The save methods in Article and Publication can call back to methods in the ArticleStore interface, this would not need changes in EntityManager if a new subclass is added to the hierarchy.

Edit Updated to reflect comment.

Answer 5

If you really want an OO design for your second issue, you can look up the visitor pattern, though that will probably be a major over kill for such a small issue, and using instanceof will get the trick done more easily.

Answer 6

When ever I hear or see trouble with inheritance, I think about using composition. In your case, an alternate design could be

+-------------+             +---------+
| Publication |--- has-a ---| Article |
+-------------+             +---------+

So the Publication class would have a field like Article articleReference and all fields needed to model the publication. Saving an article will not interfer with publications, saving a publication will save the publication and the related article.

The Publication editor could extend the Article editor and add the required publication GUI fields.

The drawback is that loading an Article including it's publication data requires a few more lines of code.

Edit

I suggest, you rename the 'Article' class because it can be mistaken for a written article as a part of a publication. My example uses 'Article' exactly as described in the question, as some sort of master data model for all kind of items, including publications.