0

Java noob here so please treat me gently. I have plenty of experience with duck-typed languages like Python and Ruby, but not much experience with statically-typed languages, and effectively zero Java experience.

I have an array of objects which are all instances of the Cause class (i.e. ArrayList<Cause>). I would like to know if any of those objects have a property upstreamRun matching a particular value. The problem is that only UpstreamCause objects have this property (UpstreamCause is a subclass of Cause), and not all of the objects in my array are UpstreamCause instances.

Here is what I first tried:

causes.stream().anyMatch((Cause c) -> c instanceof hudson.model.Cause.UpstreamCause && c.getUpstreamRun() == run);

of course, this fails to compile because upstreamRun() is not defined on the Cause parent class:

[ERROR] /home/jay/local/data/sis/workspace/henlo-world-plugin/src/main/java/org/jenkinsci/plugins/sample/HelloWorldAction.java:[17,138] cannot find symbol
  symbol:   method getUpstreamRun()
  location: variable c of type hudson.model.Cause

I tried a few other approaches as well, but all of these failed to compile:

causes.stream().anyMatch((hudson.model.Cause.UpstreamCause c) -> c instanceof hudson.model.Cause.UpstreamCause && c.getUpstreamRun() == run);

and

causes.stream().anyMatch((Cause c) -> c.getClass().getMethod("getUpstreamRun", null) != null && c.getClass().getMethod("getUpstreamRun").invoke(c) == run);

I know I'm probably approaching this the wrong way because, from what I understand, Java devs consider reflection and instanceOf to be code smells. So what's the right way to go about this?

If this were Ruby, I would do something like this:

causes.any? do |c|
  c.getUpstreamRun() == run
rescue NoMethodError => e
  false
end

But I'm guessing this kind of approach is shunned in Java, and even if it isn't, I have no idea how to write the equivalent in Java.

jayhendren
  • 4,286
  • 2
  • 35
  • 59
  • `if (cause instanceof UpstreamCause) { UpstreamCause upStream = (UpstreamCause)cause);}` – MadProgrammer Jul 31 '18 at 23:14
  • 1
    You need to cast it. But yeah, it has a bad smell. – shmosel Jul 31 '18 at 23:15
  • Is there a way to get rid of the code smell? Since I'm new to Java I'd like to know what the clean way to do this is before committing to the smelly way. – jayhendren Jul 31 '18 at 23:16
  • Unrelated, but hopefully in Ruby you'd use "respondTo" instead of forcing the exception. – Dave Newton Jul 31 '18 at 23:18
  • Thank you for the duplicate links @Radiodef! I was not able to find those by searching. I think I just do not know the right search terms to find those questions on my own. – jayhendren Jul 31 '18 at 23:18
  • @jayhendren You should probably question why you are throwing things into a collection that has a less specific type than what you might require to pull out of it. If `Cause`s and `UpstreamCause`s need to be handled in different ways, they should be in different collections. More likely, you're performing operations **upon** objects from the outside when it should really be the responsibility of the object to do the work itself (i.e. do not *get* the property from the class, but *use* the property within the class) – Michael Jul 31 '18 at 23:19
  • @Michael thanks for the suggestion but I'm getting the array of Causes directly from the Jenkins API, I'm not building it myself. – jayhendren Jul 31 '18 at 23:22
  • 1
    @jayhendren Then you have to cast it because their API is badly designed – Michael Jul 31 '18 at 23:23
  • If you're using a library and you have to use a mixed list of these objects, then there might not be much you can do besides the cast. The mistake might be in designing an API in the first place where you need to call methods on the subclass sometimes. There are some principles like, "tell, don't ask" and inversion of control that help avoid this kind of problem in the design stage. – Radiodef Jul 31 '18 at 23:25
  • @Michael @Radiodef do you mean casting like this? `List causes = b.getCauses();` Or did you mean casting on the individual objects in the array like was suggested elsewhere in these comments? ([Here](https://javadoc.jenkins-ci.org/hudson/model/Run.html#getCauses--) is the API method I'm using - it just returns `List`). – jayhendren Jul 31 '18 at 23:37
  • 1
    @DaveNewton 5 years of Ruby and I didn't know that method existed... :-/ The begin...rescue method was the first way I learned, so it's what I've always used, but `respond_to?` is definitely better. Thanks. – jayhendren Jul 31 '18 at 23:57

0 Answers0