6

I have the following code

Map<Account, Integer> accountsMap = new Map<Account, Integer>();
Account a = new Account();
accountsMap.put(a, 1);
a.name='Bob';
//System.debug(accountsMap);
System.debug(accountsMap.get(a));
System.assertEquals(1, accountsMap.get(a));

Debug in the sixth line shows null which is expected and next assert fails. However if I uncomment debug in the fifth line I will get the following log(note that there is no assert failure):

enter image description here

Which is a bit unexpected for me as Accounts with different fields should result in a different hash. I know that using sObjects for map keys is bad practice but still interested in how adding a debug statement can change the code behavior?

As noticed in the comments, to reproduce this you have to set the debug level for Apex to anything below FINEST. On FINEST level assert always fails.

  • if I both comment 5 and 6th debug line, assert fails. Also, if I uncomment any of the 5 or 6th line, assert fails. – Santanu Boral Mar 22 at 11:48
  • @SantanuBoral thank you for the check. I've tried changing debug level and when I set Apex level to FINEST it also fails with all debug statements uncommented, however on lower levels (FINER/FINE/DEBUG/...) it still passes the assert. – Oles Malkov Mar 22 at 11:57
9

Which is a bit unexpected for me as Accounts with different fields should result in a different hash. I know that using sObjects for map keys is bad practice but still interested in how adding a debug statement can change the code behavior?

Map and Set have internal state you cannot directly observe. Internally, they look a bit like this:

class Bucket<U> {
  Integer hashCode;
  U[] values;
}
public class Map<T, U> {
  List<Bucket<U>> buckets = new List<Bucket<U>>();
  public U put(T key, U value) {
    Integer hashCode = value.hashCode();
    Bucket<U> temp;
    for(Bucket<U> bucket: buckets) {
      if(bucket.hashCode == hashCode) {
        temp = bucket;
      }
    }
    // hash not found, make new bucket
    if(temp == null) {
      buckets.add(temp = new Bucket<U>());
      temp.hashCode = hashCode;
    }
    for(Integer i = 0; i < temp.values.size(); i++) {
      if(temp.values[i].equals(value)) {
        U result = temp.values[i];
        temp.values[i] = value;
        return result; // Returns old value
      }
    }
    temp.values.add(value); // Adds new value to bucket
  }
}

Of course, this isn't the actual code that happens, just sort of pseudocode. As you can see, a lot of stuff is going on under the covers.

The main point here is that if you change the value, and thus its hashCode, it will no longer be found in its original bucket, which is cached inside the map.

When you force a System.debug, the internal state of the Map is refreshed and buckets are recalculated. This can change the number of keys internally and end up with fewer total elements as well.

This occurs with both Map and Set objects. If you choose to modify the hashCode of a value for a key, you will corrupt the collection's internal state until you debug it, which fixes it.

As you've observed, this causes problems when you insert sObject records, or later modify any of their fields, or otherwise use objects that have unstable hashCode values.


Side note: Using sObject keys is not a Bad Practice™. In fact, I use this technique fairly often. sObject keys allows you to perform certain types of checks efficiently without wrapper classes. However, using this technique means you need to consciously make decisions to avoid corrupting the internal state of the collection.

  • When you force a System.debug -- this doesn't have an effect at least in this case. What OP has currently never returns value for me, which is the expected behavior at least per the documentation. – Jayant Das Mar 22 at 14:09
  • I have the exact code as OP has with no debug statements commented and it fails as expected. Only when I switch adding the name before adding the object in map, and with all other code as-is, it works perfectly fine. I was using anonymous window. – Jayant Das Mar 22 at 14:16
  • @JayantDas Sorry, just realized that this code in the example results in a null value. That's just how things get "lost." It's entirely possible that the map will partially or fully fix itself depending on its contents. Try adding a key where name = null, then another where name = bob, then change the account where name = null to name = bob, and then debug. You'll see a different behavior. – sfdcfox Mar 22 at 14:17
  • @sfdcfox Thanks, that is what I was looking for. Now I'm just curious is there any other way to "refresh a map state" other than System.debug? I assume this can be considered as a "hack" and one should not use such things in production code? Also, you said that you are using sObjects as map keys so how do you ensure their hash is not spoiled, I assume just do not modify the key once it was put in the map? Thanks – Oles Malkov Mar 22 at 14:50
  • @OlesMalkov (a) JSON.serialize I think also "fixes" a map/set (b) it is undocumented behavior, certain a "hack" to avoid in production, (c) I always construct a new "key" before using get/set; I make sure to never modify the original key material. See this answer for an example usage of this technique. Notice how I consider the keys immutable by always constructing new keys every time I need one. – sfdcfox Mar 22 at 14:55
1

UPDATE

The below answer is applicable only when the Log Level for Apex is set to FINEST.

The behavior as noted by OP does have an impact if Log Level is set anything below FINEST.


Here's my observation.

If I run the exact code you have where you are adding the Name after adding the object in Map, I never get the debug returning 1.

accountsMap.put(a, 1);
a.name='Bob';

I get it as:

08:57:25:005 USER_DEBUG [5]|DEBUG|{Account:{Name=Bob}=null}

Now, this is the expected behavior based on documentation (emphasis mine):

Be cautious when using sObjects as map keys. Key matching for sObjects is based on the comparison of all sObject field values. If one or more field values change after adding an sObject to the map, attempting to retrieve this sObject from the map returns null. This is because the modified sObject isn’t found in the map due to different field values.

Things work as expected if you will have any system.debug or not, only if you have it added as this:

a.name='Bob';
accountsMap.put(a, 1);

As noted in the documentation, exercise caution while using SObjects as keys for maps.

  • Just because there's a warning about using them doesn't mean it's not a good practice when used appropriately. – sfdcfox Mar 22 at 14:04
  • If you are too sure that your object does not change, then yes. But in scenario as this one where the field value was added after the key was added, it gets difficult to maintain. Good Practice anyways is a very much debated thing as far as I can say. I would rather frame practice as consideration as noted in the documentation. – Jayant Das Mar 22 at 14:05
  • No, I'm just saying that the documentation doesn't use the word "practice" at all. It's neither a Good Practice nor Bad Practice, just that some caution must be taken when using this technique. It's the same as with C/C++'s "pointers," a lot of people say its a "bad practice" to use pointers because they can crash the code, while others, like me, simply state you shouldn't be using them if you don't know what you're doing. The technique is perfectly safe and usable as long as one understands what goes on under the hood. The Bad Practice here is using it the wrong way. – sfdcfox Mar 22 at 14:12
  • Agreed. I actually edited the verbiage in the answer. Ah, you remind me about the pointers -- was one of my favorites :) – Jayant Das Mar 22 at 14:13
  • 1
    @sfdcfox, if I was writing Java still (rather than Apex) I would never use a mutable object as a key in a map. This really is bad practice because you open your application up to hard-to-find and hard-to-fix bugs. It is great that the Salesforce docs actually state caution should be taken. IMHO it would have been far better if Apex could make an object used as a key in a map (including an SObject) immutable. Any subsequent attempt to write to fields of that object would immediately throw an exception and prevent difficult to trace issues occurring later. Or provide a general immutable lock. – Phil W Mar 22 at 22:25
0

Note that you are using SObject as a key; when used in this way the key evaluation is based on a value comparison, so the state of your SObject is important.

When you insert into the map, the SObject was in one state, but when you get the value you have already changed the state. Whilst what you are seeing seems like strange behaviour, it is (as per @Jayant Das's answer) documented. However, what you are doing is really dodgy (IMHO).

For me, it is a bad idea to use an SObject (or indeed other mutable object) as a key; a map's internal tree/bucket structure depends on the key values and changing the key value under its feet will make the map misbehave. Much as you are seeing.

You would be better off finding a different means to provide a key for the object you are managing in the map. One option is an external ID if you must manage data in the map for SObjects that have no ID (because they are new and not yet inserted). Another is to leverage a combination of other fields within the SObject that provide some unique identity and that do not change. A third option is to hold the SObjects in an array and use the index into that array as the key value.

Whatever you do, the key value for the given SObject should not be allowed to change after first use in the map.

  • Hey Phil, thanks for your answer but as I said in my question I know that this is a bad practice. I have just used the above example as it is a good illustration of this System.debug issue and I'm not looking for any workaround. – Oles Malkov Mar 22 at 13:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.