• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Tim Cooke
  • paul wheaton
  • Jeanne Boyarsky
  • Ron McLeod
Sheriffs:
  • Paul Clapham
  • Liutauras Vilda
  • Devaka Cooray
Saloon Keepers:
  • Tim Holloway
  • Roland Mueller
Bartenders:

Using Optional<T> for improving code

 
Ranch Hand
Posts: 658
2
Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I am saving a Topic entity which has only two properties - ID(PK) and name(unique). While saving the entity I need to check if that entity already exists, and if it does then no action is required or else I need to save that entity. This is the code:

I am curious to know how can I implement this if the method signature is using java8 Optional.

And will it make any difference?
 
Puspender Tanwar
Ranch Hand
Posts: 658
2
Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
This is what I implemented
but I can't see any improvement in the code quality as here also I need to check the topic's presence using isPresent(), which I was earlier doing using topic != null
 
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Optional aside, I'm a bit confused about this test.

It looks to me like the test does not have a guaranteed starting setup (that is the Topic may or may not already exist in the test environment).
That strikes me as a a bit odd.
 
Marshal
Posts: 80238
424
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Also, how are you changing the findByName() method to change from returning Topic to Optional<Topic>? What is wrong with a predicate method to test whether that Topic already exists?
 
Bartender
Posts: 15737
368
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Why not just save the entity and if there's already an entity with that name, you get an exception. That will save you a round-trip to the database.

By the way, why does the entity have an ID if it already has a unique name? The ID is superfluous and just plain cumbersome.
 
Puspender Tanwar
Ranch Hand
Posts: 658
2
Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote:Optional aside, I'm a bit confused about this test.

It looks to me like the test does not have a guaranteed starting setup (that is the Topic may or may not already exist in the test environment).
That strikes me as a a bit odd.

I am building a REST service for a Q/A website. When users ask question, they can tag them with keywords(like java for my question here). So if the tagged keyword is already under the Topic table, then the question will be tagged to that topic otherwise the tag will be created under topic table and then the question will be tagged with that. I hope the requirement is understood.

Campbell Ritchie wrote: Also, how are you changing the findByName() method to change from returning Topic to Optional<Topic>? What is wrong with a predicate method to test whether that Topic already exists?



Stephan wrote:Why not just save the entity and if there's already an entity with that name, you get an exception. That will save you a round-trip to the database.  

Will try it. BTW, is it a good practice to handle such requirements?

Stephan wrote:By the way, why does the entity have an ID if it already has a unique name? The ID is superfluous and just plain cumbersome.

The Ids for Topic are surrogate integer keys generated by MYSQL. It will be easy to index the surrogated keys w.r.t string values (just my thoughts). Would love to here your thoughts.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Puspender Tanwar wrote:I am building a REST service for a Q/A website. When users ask question, they can tag them with keywords(like java for my question here). So if the tagged keyword is already under the Topic table, then the question will be tagged to that topic otherwise the tag will be created under topic table and then the question will be tagged with that. I hope the requirement is understood.



But this is a test case.
It should have a predetermined start point, so either (for the purposes of the test) Topic already exists in the DB or it doesn't.
There should be two tests (at least).  One for where the Topic already exists and one for where it doesn't, to ensure that the code handles both cases.
 
Puspender Tanwar
Ranch Hand
Posts: 658
2
Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Dave Tolls wrote: one for where it doesn't

But my test is for this condition only. Situation for 'topic' not exists.
 
Dave Tolls
Rancher
Posts: 4801
50
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
So I would assume the test would be:

The part in your test where you then add a Topic makes no sense.
Why is the test adding a Topic when the idea is that the test is testing what happens when a Topic does not exist?
 
Sheriff
Posts: 17734
302
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Dave's comments are spot on. OP needs to figure out what behavior he's actually testing.

Puspender Tanwar wrote:But my test is for this condition only. Situation for 'topic' not exists.



You test behavior of the system, not the condition. This is the problem with writing tests after-the-fact, you can easily get confused about what you're actually testing. That confusion is reflected by the code in your test method and by the questions/comments that Dave posted.

Here are the problems with the way you wrote that test:

1. The name ifTopicNotFound_then_InsertThatTopicWithGivenName does not convey intent very clearly. Instead, it is worded in a way that describes a conditional action.  

The intent seems to be to verify that a topic is inserted if its name does not already exist.  If I were to make the test's intent clearer, then I would rename that to something like: "topic_should_be_inserted_when_name_not_found"

2. Per Dave's comments, the test should do something like this:

First ensure that the "find topic name" part of the flow is set up to result in the "name not found" condition. This is your test setup or "Arrange".

Second, invoke the method under test. This is the "Act" portion. Your test code right now invokes findByName() and save() methods. Which of these is the method under test?

Third, the "Assert" portion: assert the condition(s) or state of the system that you expect after the Act part.

A more coherent test might look like this instead:

A test like this tells me a few things:

1. The dao.save() method is the method under test
2. It returns an Optional<Topic> that represents the Topic that was successfully saved.
3. The successfully saved Topic will have a non-null ID field
 
Junilu Lacar
Sheriff
Posts: 17734
302
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Looking at that test name again and the test code, I would also refactor the name to topic_should_be_created_when_save_cant_find_its_name() to completely and accurately reflect what's happening in the test code.

The test name and the test code should be in agreement with each other. They should tell the same story, the name at the high level and the code at a lower level of abstraction.
 
Ranch Hand
Posts: 235
5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:By the way, why does the entity have an ID if it already has a unique name? The ID is superfluous and just plain cumbersome.



This is what jumped out at me first.  As it is, it will fail on those rare names, like Robert Smith, that happen to be the same yet have different user ids.  I would think a PK of ID and name combined would be called for, but that's just me.

Regards,
(the one and only) Robert Smith
 
Climb the rope! CLIMB THE ROPE! You too tiny ad:
Smokeless wood heat with a rocket mass heater
https://woodheat.net
reply
    Bookmark Topic Watch Topic
  • New Topic