0

I have two readers implemented in JAVA. See below:

public final class ReaderA {
  public ReaderA() {}

  public static int read(final File file) {
    final byte[] data = Files.readAllbytes(file.toPath());
    return read(data);
  }

  public static int read(final byte[] data) {
    // do somethingA
  }

  ...// and some other methods
}


public final class ReaderB {
  public ReaderB() {}

  // this method is exactly the same as in ReaderA
  public static int read(final File file) {
    final byte[] data = Files.readAllbytes(file.toPath());
    return read(data);
  }

  // this is implemented different from the one in ReaderA
  public static int read(final byte[] data) {
    // do somethingB
  }

  ...// and the same other methods as in ReaderA
}

Question. What's the best way to avoid the duplicate code ?

I tried to extract a the duplicated code in a new abstract class Reader and tried to make the read(final byte[] data) abstract and implement it in subclasses ReaderA and ReaderB. It will not work because the methods are static.

cehptr
  • 157
  • 1
  • 7
  • how would making it abstract work? – Stultuske Jan 04 '18 at 09:27
  • Why did you try to make the `read(final byte[] date)` method abstract? I think you want to extract duplicate code so this method cannot be abstract because you have the (common) implementation! – Niklas P Jan 04 '18 at 09:28
  • Removing static, but I would not do this because there would be a lot of changes in the entire code where the method `read(File)` is called. – cehptr Jan 04 '18 at 09:28
  • `read(final byte[] data)` is not the same in both classes @NiklasP – cehptr Jan 04 '18 at 09:30
  • 2
    Why would those methods need to be static? – Ward Jan 04 '18 at 09:30
  • You should think about using [Guava's `ByteSource`](https://github.com/google/guava/wiki/IOExplained): it abstracts the whole read for you. – Olivier Grégoire Jan 04 '18 at 10:37

5 Answers5

2

Unless you remove the static modifiers from read(byte[]) and create instances, you won't be able to use inheritance to help you.

static methods do not behave like instance methods and can't be overriden. Instead, both the super class and the sub class will have seperate methods that need to be qualified with the class name. Pulling up read(File) means it always calls read(byte[]) of the super class. You still have to copy read(File) to each sub class to make it use that class's own read(byte[]). The code in Ward's answer shows this, too.

For reference, read this question and its answers: Are static methods inherited in Java?

To illustrate: The two read(File) methods you have are not "exactly the same" as you said in your code snippet. They do not both call this.read(data), but instead ReaderA.read(data) and ReaderB.read(data), respectively. See how the read(byte[]) calls are to two entirely different, non-overridable methods.


If it is in your power, I would recommend re-writing the readers in a less static way:

interface Reader
{
    default int read(File file)
    {
        final byte[] data = Files.readAllbytes(file.toPath());
        return read(data);
    }

    int read(byte[] data);
}

class ReaderA
    implements Reader
{
    int read(byte[] data)
    {
        // do somethingA
    }
}

class ReaderB
    implements Reader
{
    int read(byte[] data)
    {
        // do somethingB
    }
}

Notice how read(File) now is the same for all implementing classes. Naturally, you have to change the way the methods are called from ReaderX.read() to new ReaderX().read().

Malte Hartwig
  • 4,477
  • 2
  • 14
  • 30
  • they are inherited the same way AFAIK, at least the byte code is not any different than a virtual method. To me, this sounds like the `JVM` would be able to allow overriding them, but the compiler does not – Eugene Jan 04 '18 at 10:32
  • Thanks for the insight, Eugene. I re-phrased that sentence a bit. – Malte Hartwig Jan 04 '18 at 10:35
  • np, this has bugged me (still is) for some time now. good edit btw! – Eugene Jan 04 '18 at 10:37
1

I think theres a couple questions that you need to ask yourself first:

  1. Do I really need two static methods?
  2. What code is common to both 'methods' or classes?
  3. Would making an abstract class to write such implementations and make ReaderA and ReaderB extend it, reduce the duplicate code?
  4. Should you have a parent class and then create classes that inherit from the above?

I think you should take a read at SOLID principles, specifically Open/Closed Principle and Dependency Inversion

abr
  • 2,071
  • 22
  • 38
0

If you're using Java SE 8, you can put your static method in an interface. Java interface static method is similar to default method except that we can’t override them in the implementation classes. This feature helps us in avoiding undesired results incase of poor implementation in implementation classes.

amineT
  • 76
  • 5
0

There are some possibilities:

  1. Remove the method from one of the classes and use inheritance (you can't call super.myMethod() for a static method, but unless you override it, it 'll work)

I would recommend against it: you might get other methods available in your sub class, which you may not want to.

  1. Extract it to a common super-class for both classes

  2. Call the method from one class from the other. This 'll work if they both remain the same functionality

Stultuske
  • 9,296
  • 1
  • 25
  • 37
0

I'm not sure keeping this implementation with static methods is the best you can do, but if so, you could add a Function parameter.

public class Reader {

    public static int read(final File file, Function<byte[], Integer> reader) {
        final byte[] data = Files.readAllbytes(file.toPath());
        return reader.apply(data);
    }

}

And then use it like this:

public final class ReaderA {
    public static int read(final File file) {
        return Reader.read(file, ReaderA::read);
    }
    public static int read(final byte[] data) {
        // do somethingA
    }
}

Since the introduction of functional interfaces and method references in Java 8, there are hardly any duplicate code parts that cannot be avoided.

Ward
  • 2,799
  • 20
  • 26