• Post Reply Bookmark Topic Watch Topic
  • New Topic

Lambda and Predicate questions  RSS feed

 
Paul Peterson
Ranch Hand
Posts: 106
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm playing with Lambdas and getting a little frustrated.  First, I haven't been able to find much detailed info about Lambdas online.  I'm sure some of you can cure that problem.  I did find the videos by Java Brains.

What I'm trying to do is write a method that allows the user to search a list DVD's by a certain field in the DVD object.  I am trying to pass the predicate in the method call.  One area that I am having trouble is with the Consumer.  In some cases I just want to print the title and in others I want to show all of the DVD information.  I don't want to hard wire each method call for both cases.



While I was working on this I resolved the Consumer issue, but the search results is still returning an empty list. Case 1 is working, but cases 2 and 3 are returning an empty list.

Also, is there a quick way to filter a string ignoring case using streams?  In this code, I forced the user input to lowercase added the toLowercase in the contains part of the stream.  Is there a better/easier way?
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 37469
539
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I see two problems. First you predicate isn't correct. You need to lower case both expressions so they are equivalent:


Second, you use "result" as an instance variable for two different things. This highlights that the following isn't great style:


Instead, you can use a built in method to convert to the result:
 
Paul Peterson
Ranch Hand
Posts: 106
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If I'm reading you correctly I could use



to replace the two method calls I had in lines 22, 23 and 32, 36, right?
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 37469
539
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No. Lines 65-66. This is getting rid of the "forEach" that adds to your result list.
 
Paul Peterson
Ranch Hand
Posts: 106
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ok, thank you for your help!
 
Carey Brown
Saloon Keeper
Posts: 3322
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Should even be able to make it slightly shorter
 
Rob Spoor
Sheriff
Posts: 21135
87
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I was worried I was the only one that found the wrapping of a predicate inside another predicate (lambda) odd.
 
Paul Clapham
Sheriff
Posts: 22828
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have to admit that I've written an anonymous inner class which contained another anonymous inner class, so I wouldn't balk at a predicate inside a predicate, but I do agree that it could be difficult to understand.
 
Carey Brown
Saloon Keeper
Posts: 3322
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

It looks like, in your above code, that you are applying the searchByTitle predicate twice, once on line 22 and again when you call displayResults(). Seems like line 22 is redundant.
 
Rob Spoor
Sheriff
Posts: 21135
87
Chrome Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:I have to admit that I've written an anonymous inner class which contained another anonymous inner class, so I wouldn't balk at a predicate inside a predicate, but I do agree that it could be difficult to understand.

There's nothing wrong with a predicate inside a predicate (heck, that's what Predicate.negate does), but if the outer predicate is simply delegating to the inner predicate without doing anything else then the outer predicate is superfluous.
 
Junilu Lacar
Sheriff
Posts: 11490
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't know, maybe it's just me and my obsession with writing clean code but it seems to me like you're asking "Hey, is there anything on my face?" and everyone is trying to be polite and saying, "Well, you got a little bit of stuff on your cheek and a smudge on your nose..." 

Well, I'm just going to come right out and say it: "Dude, you just ate a whole skunk, your breath stinks, and it looks like you're getting ready to eat at least a couple more skunks, and maybe a possum!" 

That whole searchDVD() method stinks of complexity and bloat and you still have three more cases to implement. You'd be doing yourself a huge favor if you stopped and tried to clean that up and pare it down by extracting some chunks of code to their own methods. You said in another post you don't like to repeat the same thing over and over. Well, you also have an opportunity to eliminate some duplication here.
 
Junilu Lacar
Sheriff
Posts: 11490
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's a Tic-Tac to get you started...

One smell from that searchDVD() method is that you've mashed together all the logic for displaying a search menu, dispatching the menu selection to handlers, and actually doing the work the user requested. That stinks. An analogy in biology would be if we had the functions of our heart, eyes, stomach, and intestines all mashed up in one organ. It'd be a mess and it wouldn't work very well. Kind of like how that method is right now.

Start by defining a high-level method that describes the general flow, something like this:

I'd try making SearchOption an enum type, maybe have each option encapsulate some search criteria, maybe make it so that SearchOption.getPredicate() returns the appropriate lambda expression for searching by, say DIRECTOR or whatever.

Try to keep your methods small and focused on one job, like how the Hershel Greene character on "The Walking Dead" used to say all the time: "Everyone has their job to do."
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!