• 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
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Which version is better practice

 
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hello, a small dilemma, which version do you prefer? Why? Know any situations where one has advantages/disadvantages over the other?

Mock simplified examples:

The use of each.

Version1:

Version2:

Thank you for insights.
 
Saloon Keeper
Posts: 15485
363
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You want constructors to be as simple as possible, ideally just doing parameter checking and setting properties. If the string is the value of some property, use the constructor directly. If properties must be... well... parsed from the string, use the factory method. Keep processing out of constructors.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks Stephan
 
Sheriff
Posts: 17644
300
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
I'm with Stephan on this. Looking at the evolution of some classes in the Standard Library, it seems like some of the maintainers are leaning the same way as well. Take the old Date class. Its Date(String) constructor has been deprecated in favor of DateFormat.parse(String).

On the other hand, Integer(String) has been around forever. I'd still go by Stephan's rule of thumb though.
 
Junilu Lacar
Sheriff
Posts: 17644
300
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 more closely at your examples though, seems like you don't even convert the String to some other type.  Also, the getSomething() method returns a String so the parse method is misleading. Not sure if you just overlooked that in your example or if that was the intended design.  If the latter, then Version2 is misleading because you're not actually parsing the value.  Version1's constructor parameter name, textToParse, is also misleading because you're not actually parsing anything.

The code you gave has Version1 with the constructor that takes a String while Version2 has the parse() method. Your usage example has switched them around. Maybe you should revise your code and example usage so that we can see what you actually intended. Right now everything looks wrong.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Well, even as for an example I should have better reflect data types and better mimic actual cases. My apologies.

The parameter which is named as textToParse is actually a JSON string, which gets parsed into a JsonObject. Later on this object is meant to be used to extract various attribute-value pairs.

So the revised versions to reflect better actual scenario would look like:

The thing is liked about the Version1 is, that regardless of how JSON object will grow in a future, the change is just the matter of adding extra getter.

In case of Version2, there would be a need to modify private constructor, add extra field, add getter if the arguments would be passed after they get extracted. If to pass just parsed JsonObject and later extract attributes in the same way as in Version1, then again, Version2 looks somewhat more complicated to me with not much benefit.

I hope now I was a bit clearer. However, please ignore variables naming as well as lack of validation - I just took these out of my head to visualise idea.
 
Stephan van Hulst
Saloon Keeper
Posts: 15485
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Why doesn't your constructor take a Map? That's what it needs to initialize the class. Not a String.

Woops I got confused. Ignore me.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Probably I also need to consider the difference between the versions, that:

Version1 extracts the attribute/-s upon the need, while Version2 would extract them during Json string parsing if I decided to construct object with extracted and ready to use all attributes which are passed to a constructor.
 
Sheriff
Posts: 5555
326
IntelliJ IDE Python Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When I have dilemmas like this I say to myself "What would Josh do?", and reach for my copy of Effective Java. It says:

Effective Java wrote:One advantage of static factory methods is that, unlike constructors, they have names


Effective Java wrote:A second advantage of static factory methods is that, unlike constructors, they are not required to create a new object each time they're invoked


Effective Java wrote:A third advantage of static factory methods is that, unlike constructors, they can return an object of any subtype of their return type


Effective Java wrote:A fourth advantage of static factories is that the class of the returned object can vary from call to call as a function of the input parameters


Effective Java wrote:A fifth advantage of static factories is that the class of the returned object need not exist when the class containing the method is written


Effective Java wrote:The main limitation of providing only static factory methods is that classes without public or protected constructors cannot be subclassed


Effective Java wrote:A second shortcoming of static factory methods is that they are hard for programmers to find



Like Junilu says, the method and parameter naming is misleading so it's hard to know what the right approach is.

Here's a consideration that I might have coming from a performance point of view. GC 'stop the world' events are bad, so one way to avoid them is to create zero garbage so in your example the Version class is immutable and we could create a cache for it. Yes the trade off is memory usage, but in my world that's ok. The static factory method is perfect here:


But, it depends on the detail, usage, and what's important for you.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Liutauras Vilda wrote:If to pass just parsed JsonObject and later extract attributes in the same way as in Version1, then again, Version2 looks somewhat more complicated to me with not much benefit.


Over here I had in mind this, and this is probably what Stephan suggested in his initial reply


Reading all replies once again...
 
Junilu Lacar
Sheriff
Posts: 17644
300
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
Sorry, Liutauras, but the names are supposed to help the reader understand the idea being represented in the code. The names in your example are giving mixed-signals and thus making it difficult to understand your intent.

You declare the field JsonObject something but then your accessor is declared as Map<String, String> getSomething() -- a bit of a Stroop Effect going on there.

It gets even more confusing when you have Map<String, String> get(String attribute) -- I can't get the concepts straight between having a vague name like something and calling something an attribute that is related in some way to the something field.  

If that previous sentence sounds confusing, it's because it reflects the state of my understanding of what the code is trying to say.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Sorry, Liutauras, but the names are supposed to help the reader understand the idea being represented in the code.


Good grief, I was really that terrible in representing my question.

How about now?
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
So the initially posted version 2 would look now with improved naming like:


And the version 3 would look like:

Sorry once again for the created confusion.

 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Please feel free to disregard this topic, I'm experimenting more in order to have more implementation details to consider, so maybe the answer will become apparent.

I think I was too early to try to make decision + created all that mess with naming which didn't help either.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
On the contrary, I think it's useful to study how an idea evolves from a state of jumble and confusion and then evolves to have clarity and better organization. It's fascinating to me to see patterns of thought that other people go through and very satisfying to see how a consensus and common understanding is reached through discussion and logical reasoning.

The great thing about some mistakes is that they can teach us so much.
 
Bartender
Posts: 1868
81
Android IntelliJ IDE MySQL Database Chrome Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Mooo!

Your posting was just mentioned in the June 2018 CodeRanch Journal and for that you get a cow.
 
Ranch Hand
Posts: 45
8
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If the only purpose of the class is to return a map of column titles from a json String, why have a class at all? Just code it in a single method.

Other the other hand, if the purpose of the class to provide a Map from a json String, why not have the class implement Map<>?
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Pete Letkeman wrote:Mooo!

Your posting was just mentioned in the June 2018 CodeRanch Journal and for that you get a cow.


Thank you.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Gerard, thanks for comments.

Gerard Charles wrote:If the only purpose of the class is to return a map of column titles from a json String, why have a class at all? Just code it in a single method.


Well, this personalization thing is meant to grow, currently it just column titles (in example), but there will be more stuff (is already). So I think have it separated is not bad just not to cram it to random class.

...if the purpose of the class to provide a Map from a json String, why not have the class implement Map<>?


I'm not implementing Map in this case, I'm parsing data which output appears to end up as Map data structure. But not defining what the put method or remove might do, so don't think implementing like that would be the right go.
 
Gerard Charles
Ranch Hand
Posts: 45
8
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
put and remove are optional methods, so you don't have to implement them. Java provides an umodifiable map API to make it easy to provide read only maps.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Gerard Charles wrote:put and remove are optional methods, so you don't have to implement them. Java provides an umodifiable map API to make it easy to provide read only maps.


Maybe, but the idea to implement map when I'm not creating one is kind of not feeling right.
 
Everyone is a villain in someone else's story. Especially this devious tiny ad:
a bit of art, as a gift, that will fit in a stocking
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic