views:

918

answers:

5

In this code :

public class MyClass {
    private Object innerValue;
    public Object getInnerValue() {
        return this.innerValue;
    }
    public void setInnerValue(Object innerValue) {
        this.innerValue = innerValue;
    }
}

public class MyClassReadOnly extends MyClass {
    MyClassReadOnly(MyClass cls) {
        // Make a field by field copy
        super.setInnerValue(cls.getInnerValue());
    }
    public void setInnerValue(Object innerValue) {
        throw new UnsupportedOperationException(
                            "This is a read-only instance"
                        );
    }
}

The compiler complains rightly about the unused parameter(never read) innerValue in MyClassReadOnly.setInnerValue().

I don't want to disable this kind of warnings since it's quite useful usually, and I don't want to have any warnings either to have a high signal/noise ratio.

I cannot use the @SuppressWarnings() construct as another question suggested since it's Java 1.4 only.

I thought about inserting dummy code like this, but it's not very satisfactory :

public void setInnerValue(Object innerValue) {
    if (innerValue != null) { /* Do Nothing, but keep the compiler happy */ }
    throw new UnsupportedOperationException("This is a read-only instance");
}
A: 

I'm afraid you're stuck with dummy code. In C/C++, you could use a macro (#define _unused(x) ((void) x)), but (void) variable; isn't a valid statement in Java.

If it makes you feel better, the compiler will likely optimize away the empty if-block.

Michael Myers
That answered my question (so +1 to counteract the -1), but Uri's answer is better, although not stricly applicable in my case (almost the same pb as the standard java collections : I don't own the base class)
Steve Schnepp
A: 

You can safely enter lines like: innerValue = null; at the top of the function for all unused args. It will not affect the caller, but will keep the compiler happy.

an0nym0usc0ward
That won't be optimized away, but a. optimization of a method that shouldn't be called isn't a big deal, and b. it's shorter than "if (innerValue == null){}".
Michael Myers
+1  A: 

I wouldn't play any "code tricks" just to make a compiler warning go away, hoping for the compiler to optimize away the tricks. In fact, is this compiler warning all that useful? I would just disable it. Once you are using Java 5, you can use @SuppressWarnings and reenable it.

IMO, it's a bad idea to enable all possible warnings, just because they exist, and then set out to make every single warning go away. Figure out which warnings actually make sense for your environment and disable the rest.

Eddie
Every warning is useless until it shows you something you care about. So yes, it's useful. :)
Nick Veys
See the #1 answer--turns out that most warnings are VERY USEFUL, and to just recommend disabling one the coder don't fully understand is not really a good answer. Sorry for the -1 otherwise :(
Bill K
@Bill K: I greatly appreciate you telling me *why* you voted down. My answer was more general than this specific question, which is perhaps my error. I still stand by: "Don't do gymnastics in your code simply to make warnings go away." But the #1 answer is the better answer to this specific question.
Eddie
+10  A: 

The warning is not the problem, I'm afraid that the design is.

Your current hierarchy violates Liskov's principle of substitution since a class receiving an instance of a MyClass expects setInnerValue to work, and may not handle this exception correctly. You can say that a read-and-write X is a type of readable-X, but you cannot say that a readable-X is a type of read-and-writable X.

When I am faced with this sort of situation, I create an interface called IMyX with the reads, a subinterface called IMutableMyX with the writes, and then the actual class implements IMutableMyX and thus also IMyX. I am then very careful to only pass IMutableMyX when I need to, and pass IMyX in all other cases.

I feel that it is better to use the compiler and types to restrict access than it is to count on runtime exceptions. It also makes your code a lot clearer, and forces you to explicitly downcast the interface when you do want write-access.

I realize this does not answer your question about getting rid of the warnings. But warnings can either be suppressed, ignored, or addressed. An unused parameter is often a bad smell that indicates your method might not be doing what it's expected to do. Methods should only get essential parameters. If the parameter is not used, the parameter is not essential, and therefore something needs to be changed.

Uri
Wholeheartedly agree. Java set some bad precedence when they created interfaces like these with Iterable and Collections.unmodifiable*.
Travis Jensen
I agree that the Collection classes are a major problem. For example, being able to have a Map, and then call add and get an exception since it's a TreeMap and your item is not Comparable isn't exactly great.
Uri
Very good answer. +1
Michael Myers
I'm accepting it, although it answered my question in a way I didn't expect, and I share your vision of "An unused parameter is often a bad smell" (That's why I didn't just deactivate the warning). Btw, I will not use interfaces since that would be overkill, but the principle is quite attractive (Just invert the inheritance scheme if possible : ReadOnly extends Mutable)
Steve Schnepp
@Steve: Thank you. I think that disabling the warning is often best done at the IDE level (so it doesn't affect others) and at least Eclipse may allow you to do it. However, in this case I felt like you're heading for more trouble.
Uri
@Steve Depending on the size of your project, interfaces may or may not make sense. I have a group of projects that depend on a small group of core classes, I found that interfaces and especially the immutability scheme are a great help. This may also be useful if you have synchronization issues.
Uri
A: 

If you're using Eclipse (?), you can turn on Parameter Is Never Read warnings, but ignore cases in overriding and implementing methods (which would solve this specific problem) and, separately, those documented with the "@param" tag (although of course that doesn't apply to Java 1.4). I would expect most other Java IDEs to have similar settings available.

Carl Manaster