1
\$\begingroup\$

How could this be improved style-wise (if this isn't good enough) when the need is just to have a console replacement for PrintWriter.format() with backwards compatibility?

class CCConsole // not to be confused with any Java class with a similar name
{
    static PrintWriter pw = new PrintWriter(System.out, true); 

    static void redirect(LongRunningWorker longRunningWorker) {
        pw = new PrintWriter(new LongRunningWorkerWriter(longRunningWorker));
    }
}

class LongRunningWorkerWriter extends Writer
{
    LongRunningWorker longRunningWorker;

    LongRunningWorkerWriter(LongRunningWorker longRunningWorker) {
        this.longRunningWorker = longRunningWorker;
    }

    @Override
    public void close() throws IOException { }

    @Override
    public void flush() throws IOException { }

    @Override
    public void write(char[] chars, int offset, int length) throws IOException {
        this.longRunningWorker.rePublish(new String(chars, offset, length));
    }
}

abstract class LongRunningWorker extends SwingWorker<Boolean, String>
{
    ColorSwingConsole colorSwingConsole;

    LongRunningWorker(ColorSwingConsole colorSwingConsole) {
        CCConsole.redirect(this);
        this.colorSwingConsole = colorSwingConsole;
    }

    public Boolean doInBackground() // concrete for abstract
        throws Exception {
        go();
        return true;
    }

    abstract void go() throws Exception;

    void rePublish(String s) {
        publish(s); // protected final = inherit or same package, cannot override
    }

    @Override
    public void process(List<String> chunks) {
        for (String s : chunks)
            this.colorSwingConsole.append(s);
    }
}

class RealWorker extends LongRunningWorker
{
    RealWorker(ColorSwingConsole css) {
        super(css);
        // constructor stuff
    }

    void go() throws Exception {
        CCConsole.pw.format("%s\n", "hello world");
        // application stuff
    }
}
\$\endgroup\$
3
\$\begingroup\$

I did not understand what exactly are you asking here. Anyway, I will review your code:

First, make the fields private and provide getters when you need to access them. These that are set in constructors and are never re-setted, you may make them final.

Second, the CCConsole class may break if multiple threads access the redirect method at the same time.

Third, the CCConsole.redirect(this); in the LongRunningWorker is bad. There is no way to create a LongRunningWorker for testing purposes that does not interferes with everything else. Further if anything interferes with the CCConsole (which acts as a singleton), there is no way to remove it. I sugesting to make the CCConsole a normal object (i.e. you instantiate it and no more static fields used). In the LongRunningWorker constructor add a CCConsole parameter. In the RealWorker class, get a CCConsole from somewhere (instantiate it, get from a parameter, from a factory method, whatever).

Forth, avoid inheritance if you can using the strategy design pattern. Instead of making LongRunningWorker abstract with the abstract go method to be overriden in the RealWorker subclass, do this:

  1. Create an interface with the go method (lets say LongWork).

  2. Add a parameter to the LongRunningWorker class of the type LongWork, and save it in a field.

  3. In the doInBackground() method, instead of calling the local go() method, call the one from the LongWork object.

  4. LongRunningWorker is not abstract anymore, and you may remove the go() method.

  5. RealWorker does not extends LongRunningWorker anymore, but now implements the LongWork interface.

  6. You may remove the ColorSwingConsole parameter from the RealWorker class.

\$\endgroup\$

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.