0
\$\begingroup\$

I'm trying to create an extensible system for building messages.

I want each style of message to be a unique class. I don't want the client to have to worry about the class type when creating the message (or reading it – but that is not included here).

In the code below, I only use ints and Strings. The final version would wrap ints and Strings in another class with data validation if necessary.

// Run from main
import xx.xxx.StandardFactory.*;
private void standardFactoryTest() {
    StandardFactory standardFactory = new StandardFactory();

    BasePackage testPackage1 = standardFactory.makePackage("Text for One");
    BasePackage testPackage2 = standardFactory.makePackage(999);
    BasePackage testPackage3 = standardFactory.makePackage("Text for Three", 129);

    testPackage1.printData(); // prints “StringPackage value: Text for One”
    testPackage2.printData(); // prints “IntPackage value: 999”
    testPackage3.printData(); // prints “StringIntPackage value: Text for Three, 129”
}

package xx.xxx.StandardFactory;

public class StandardFactory {
    public abstract class BasePackage{
        public abstract void printData();
    }

    public class StringPackage extends BasePackage {
        private final String value;

        StringPackage(String value) {
            this.value = value;
        }

        public void printData() {
            System.out.println("StringPackage value: " + value);
        };
    }

    public class IntPackage extends BasePackage  {
        private final int value;

        IntPackage(int value) {
            this.value = value;
        }

        public void printData() {
            System.out.println("IntPackage value: " + value);
        };
    }

    public class StringIntPackage extends BasePackage  {
        private final String stringValue;
        private final int intValue;

        StringIntPackage(String stringValue, int intValue) {
            this.stringValue = stringValue;
            this.intValue = intValue;
        }

        public void printData() {
            System.out.println("StringIntPackage value: " + stringValue + ", " + intValue);
        };
    }

    public StringPackage makePackage(String value) {
        return new StringPackage(value);
    }

    public IntPackage makePackage(int value) {
        return new IntPackage(value);
    }

    public StringIntPackage makePackage(String stringValue, int intValue) {
        return new StringIntPackage(stringValue, intValue);
    }
}

Potential problems: If I add new classes I need to add them to the factory. I don't see this as too much work. Messages could take arguments in a different order. I will not need to make every different combination of input for my purposes so this should not be a problem. Large packages may need five or six different arguments. This goes against the principle of using few arguments in a function, but it would be the exception not the rule.

I am interested specifically in the following:

  1. Is this a common design pattern (is it a version of the factory pattern)?
  2. Are there more suitable design pattern options available?
  3. Are there any obvious improvements?
  4. Is it possible to have the factory static so I don't need a factory object?

I am not interested in the following (with reasons):

  1. Naming – This has been done as example code
  2. Comments – I have not attempted to comment this code
  3. Data validation – Not required at this point

Everything else is fair game.

\$\endgroup\$
3
\$\begingroup\$

Seems you are on the right way. But mostly depends how will the client use your factory and BasePackage. Here are some my minor remarks for improvements:

  • Only BasePackage is to be public, all the implementation classes are to be hidden.
  • makePackage() should return BasePackage type.
  • To reduce possible 5..6 arguments think about varargs solution.
  • Yes, you may have factory as a static, because you don't have an internal state (at least for now).
  • Naming: if possible rename BasePackage to reflect your domain area. BasePackage for client perspective may be not clear. But it is ok if you have good package hierarchy.
\$\endgroup\$
  • \$\begingroup\$ Great ideas. I'll look into those. Thank you. \$\endgroup\$ – David Jul 26 '17 at 2:46
  • \$\begingroup\$ I really like the idea of having BasePackage public and keeping the rest private. There is no need for the client to know the message type. Varargs is a good idea, but I will enclose all primatives in other classes so I don't need to worry about that. I will set everything static (including the different XXXPackages). Names will be altered. Thank you for all your ideas. \$\endgroup\$ – David Aug 1 '17 at 18:18
2
\$\begingroup\$

You mention extesibility as a goal of the project. You might consider making BasePackage an interface rather than a base class, and using composition, rather than extension to create the StringPackage IntPackage and StringIntPackage classes. That will give you much more freedom in how those classes (and future classes) are implemented.

I don't think this would change much at present. But as more XxxPackage classes are added, and more features are added, that flexibilty becomes simultaneously more important, and harder to add.

I also don't see a need to make StringPackage, IntPackage, and StringIntPackag member classes of StandardFactory. As more Package implementations are added, this code will get harder and harder to read & maintain. I suggest making them independent classes. If you're concerned about outside packages instantiating them directly, you can make the classes package-private, and only expose the BasePackage as public.

(Sidenote: there's a larger anti-pattern at work here. Any time a single file is the "right place" for lots of future changes/additions, you're setting yourself up for trouble. I only use member classes when instances need to access the internals of the parent class.)

I think your instincts are right about replacing the StandardFactory instance with a static factory method. I don't see a need here (though this is a simplified example) for

With all that done, here's the new code (note that it's split into 5 separate files):

/* file: StandardFactory.java */
package xx.xxx;// Run from main

/**
 * Provides static factory methods for Package instances.
 */
public class StandardFactory
{
    public StringPackage makePackage(String value) {
        return new StringPackage(value);
    }

    public IntPackage makePackage(int value) {
        return new IntPackage(value);
    }

    public StringIntPackage makePackage(String stringValue, int intValue) {
        return new StringIntPackage(stringValue, intValue);
    }
}

/* file: Package.java */
package xx.xxx;

public interface Package
{
    void printData();
}

/* file: IntPackage.java */
package xx.xxx;

class IntPackage implements Package
{
    private final int value;

    IntPackage(int value) {
        this.value = value;
    }

    public void printData() {
        System.out.println("IntPackage value: " + value);
    }
}

/* file: StringPackage.java */
package xx.xxx;

class StringPackage implements Package
{
    private final String value;

    StringPackage(String value) {
        this.value = value;
    }

    public void printData() {
        System.out.println("StringPackage value: " + value);
    }
}

/* file: StringIntPackage.java */
package xx.xxx;

class StringIntPackage implements Package
{
    private final String stringValue;
    private final int intValue;

    StringIntPackage(String stringValue, int intValue) {
        this.stringValue = stringValue;
        this.intValue = intValue;
    }

    public void printData() {
        System.out.println("StringIntPackage value: " + stringValue + ", " + intValue);
    }
}

I hope this helps!

\$\endgroup\$
  • \$\begingroup\$ @Chis Definitely helps. I need to spend some time thining about this before I can ask the questions I want to a \$\endgroup\$ – David Jul 27 '17 at 8:50
  • \$\begingroup\$ @David, Glad to hear it's helpful! No solution is perfect, least of all mine. But I hope it gives you food for thought. Cheers! \$\endgroup\$ – DeathB4Decaf Jul 28 '17 at 0:26
  • \$\begingroup\$ @Chis I considered a separate class for each Package, but decided to put them into a single 'Packages' class to keep everything in one place. I looked at using an Interface, but it seemed wrong. The extensions of Package are logical extensions of the class, and I would like to have some other functions in the base that will be shared by descendants. Any flaws with these choices? \$\endgroup\$ – David Aug 1 '17 at 16:46
  • \$\begingroup\$ @David Generally, if your goal is extensibility (as you mentioned in your original question), using an interface and separate classes will serve that purpose better than a base class & inheritance. Using internal classes also works against extensibility. I totally agree with your instinct to provide a clear place for new code to go, but in my own code I avoid having that place be in a single file. It encourages merge conflicts and makes that file less readable. \$\endgroup\$ – DeathB4Decaf Aug 1 '17 at 21:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.