We help IT Professionals succeed at work.

review the  java code

roy_sanu
roy_sanu asked
on
is it better to represent as String and Object or will there be any issue in the future ?
Thanks Experts sir.

@Override
      public void buildObjectMap() {
            super.buildObjectMap();
            Map<String, Object> objectMap =  getObjectMap();
            objectMap.put("id", id);
            objectMap.put("status", this.getStatus());
            objectMap.put("scheduledDate", this.getScheduledDate());
            objectMap.put("visitationPoint", visitationPoint.toJsonizedMap());
            objectMap.put("teamId", this.getTeam().getId());
            objectMap.put("teamName", this.getTeam().getTeamname());
            
            objectMap.put("locationId", this.getVisitationPoint().getLocation().getId());
            objectMap.put("locationName", this.getVisitationPoint().getLocation().getName());
            objectMap.put("locationType", this.getVisitationPoint().getLocation().getLocationType().getName());
      }
Comment
Watch Question

ste5anSenior Developer
CERTIFIED EXPERT

Commented:
There is the subject missing in your question..

Either I'm unaware of something magic going on or your method makes no sense at all:

@Override
public void buildObjectMap() {
    super.buildObjectMap();
    Map<String, Object> objectMap =  getObjectMap();
    objectMap.put("id", id);
    // [..]
    objectMap.put("locationType", this.getVisitationPoint().getLocation().getLocationType().getName());
}

Open in new window

The objectMap used in your method is not used nor passed upstream. Thus the method itself makes no sense as long as super.buildObjectMap() and the getter access is side-effect free.
In other words, this method doesn't do anything at the first glance.
roy_sanudeveloper

Author

Commented:
super is used for master record has to fetch first Otherwise we should not have used as the data structure has been defined by other team friends which I am trying to understand expert ste5an. Let me know if the other part is ok Otherwise I have to revamp the code suggest something more Expert. Is it <String, Object >
on Hashmap is good as of now? Is that ok or I can go ahead with any change in it.let me other and some other questions I have asked previously it would be nice of you in other questions which I have put forward

Thanx Roy.S
ste5anSenior Developer
CERTIFIED EXPERT

Commented:
Again: You are creating an  object, then you throw it away without using it. This makes no sense at all.

And as long as your method makes no sense, the question whether the usage of hashlist or what ever is meaningless. Thus it cannot be answered.
roy_sanudeveloper

Author

Commented:
https://www.experts-exchange.com/questions/29170969/Concern-with-JPA,

Ste5an expert please refer my url where a inbase model class it is defined well.if you have any questions to ask let us know. I am good in architecture but not good in jpa based job which looks to sometimes boring.your suggestions are good but I will not take it easy.

Best Regards
Expert.
CERTIFIED EXPERT
Commented:
In general it's best to use a more specific type if you can, so prefer

Map<String,String>
over
Map<String,Object>

if you know that only strings will be stored in this map.

The issue, which I think is part of what is confusing ste5an is that the map you're using appears to be being built in the super class.  So there may be other subclasses that are using the same map and they would all need to be changed to also use

Map<String,String>

If you change this.

Also ste5an, not really sure why you think this method doesn't do anything.  I think we're meant to assume that the map returned by
getObjectMap()
is owned by this class and so will persist beyond the method call.  There's no map creation occurring in the method itself (no call to new), so we assume I think it's being created in the super class.

Doug
ste5anSenior Developer
CERTIFIED EXPERT

Commented:
@Doug:
In the case that the map is build and stored in the super class, then getObjectMap() should be redundant. Here I would expect:

@Override
public void buildObjectMap() {
    super.buildObjectMap();
    objectMap.put("id", id);
    // [..]
    objectMap.put("locationType", this.getVisitationPoint().getLocation().getLocationType().getName());
}

Open in new window

Under normal circumstances a getter should return an immutable or a copy/clone, not a reference.. that's why I think the method itself is not self-explanatory.

And for types, it looks like a "viewbag". Thus object for the value is necessary as long as the used view concept does not consume plain strings.
CERTIFIED EXPERT

Commented:
Agreed ste5an it's an unusual/undesirable pattern to call a getter and then modify it.

But it is probably what's happening here and then at least the code makes sense, even if we can suggest ways to improve it.

So I think we're on the same page.
roy_sanudeveloper

Author

Commented:
What I would like to guess is String is good according to your concern but issue is in our database structure we are having one table as a master and another detail table called location and sub location as sublocation needs to be called by location table. As per our senior resource like you has design this as I am trying to understand it without his help as he has left our college project and left to other country. Similarly we have various zones in a state where data has to go granular so that is reason I thought you both can able help on executing some old issues. Any way good day.