function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
David81David81 

First VF page + Custom controller = governor limit error on test method

So now I'm diving head first into Visualforce and custom controllers. The driving force behind this new project is an issue that is causing Account records to be created with duplicate values in a custom field. At this point it is not an option to make this field "unique".

 

To identify these duplicates, I'm currently exporting a list of Accounts with this field populated, importing this table into Access and running a Find Duplicates query on the data.

 

Well, I thought VF might offer a more elegant solution to this problem so I tried whipping up a quick page and custom controller to display these duplicate records. I'm sure there is a more elegant way of doing this, but it was a good learning exercise for me.

 

The problem comes in the Test method. I  really only interested in Accounts created in the last 30 days, so I've limited by query by CreatedDate, but the Test method still hits a governor limit in production. Is there a quick and easy way for me to fix this?

 

 

VF Page:

 

<apex:page controller="DupeAcctNumbers"> <apex:pageBlock > <apex:pageBlockTable value="{!dupes}" var="d"> <apex:column headerValue="Account Number"> <apex:OutputLabel value="{!d.AccountNumber__c}" id="txtAccountNumber__c" /> </apex:column> <apex:column headerValue="Name"> <apex:OutputLink value="../{!d}" id="txtName" >{!d.name}</apex:OutputLink> </apex:column> <apex:column headerValue="Created Date"> <apex:OutputLabel value="{!d.CreatedDate}" id="txtCreatedDate" /> </apex:column> <apex:column headerValue="Created by"> <apex:OutputLabel value="{!d.Createdby.Name}" id="txtCreatedby" /> </apex:column> <apex:column headerValue="Owner"> <apex:OutputLabel value="{!d.Owner.Name}" id="txtOwner" /> </apex:column> </apex:pageBlockTable> </apex:pageBlock> </apex:page>

 

 

 

 

Custom Controller:

 

public class DupeAcctNumbers { private datetime tminus30 = datetime.now().addDays(-30); private Set<String> acctsLast30 = new Set<String>(); private Integer counttmp = 0; private Map<string,integer> acctnumcount = new Map<string,integer>(); private Set<String> dupenums = new Set<String>(); public List<Account> dupes = new List<Account>(); public List<Account> getdupes() { for(Account a30 : [SELECT AccountNumber__c,CreatedDate FROM Account WHERE AccountNumber__c != null AND CreatedDate > :tminus30] ) { acctsLast30.add(a30.AccountNumber__c); } for(Account a : [SELECT AccountNumber__c,Id FROM Account WHERE AccountNumber__c IN :acctsLast30]){ if(acctnumcount.containsKey(a.AccountNumber__c)) { counttmp = acctnumcount.get(a.AccountNumber__c) + 1; acctnumcount.put(a.AccountNumber__c,counttmp); } else { acctnumcount.put(a.AccountNumber__c,1); } } for(String c : acctnumcount.keyset()) { if(acctnumcount.get(c) > 1) { dupenums.add(c); } } for(Account dupe : [SELECT AccountNumber__c,Name,Id,CreatedDate,Createdby.Name,Owner.Name FROM Account WHERE AccountNumber__c in :dupenums ORDER BY AccountNumber__c]) { dupes.add(dupe); } return dupes; } //test method static testMethod void runPositiveTestCases() { Account test1 = new Account(FirstName='Test1',LastName='Test1',AccountNumber__c='12345678'); insert test1; Account test2 = new Account(FirstName='Test2',LastName='Test2',AccountNumber__c='12345678'); insert test2; DupeAcctNumbers dTest = new DupeAcctNumbers(); dTest.getdupes(); } }

 

 

 

AvromAvrom

I'm not sure whether this will solve your problem, but you're definitely doing a lot more queries than you need to--three times as many, it looks like to me. For example, instead of having

 

 

for(Account a30 : [SELECT AccountNumber__c,CreatedDate FROM Account WHERE AccountNumber__c != null AND CreatedDate > :tminus30] ) { acctsLast30.add(a30.AccountNumber__c); } for(Account a : [SELECT AccountNumber__c,Id FROM Account WHERE AccountNumber__c IN :acctsLast30]){ ... }

 

 Why not just have

 

 

for(Account a : [SELECT AccountNumber__c,Id,CreatedDate FROM Account WHERE WHERE AccountNumber__c != null AND CreatedDate > :tminus30]){ ... }

 

 

 

 In fact, if you put an order by on this clause, I think you can probably even skip that last query, just scanning through these results to find duplicates (which, with an appropriate order by, will always be adjascent).

 

 

David81David81

Thanks for the tips. I figured this would be a bit dirty on the query/list side and I'm still getting comfortable with optimizing these areas.

 

I'm pretty sure that I'll still run into the governor limits on the Test since in production we are likely to have over 500 Accounts created in the last 30 days.

 

Edit: going back and looking at it, I do need to create a list of the account numbers for accounts created in the last 30 days, then pull any accounts in our whole database that match those account numbers, as they may have been created before 30 days ago.

 

The first query is to get the account numbers for accounts created in the last 30 days.

The second looks at all of our accounts with account numbers within that set, even if they are older than 30 days.

 

Edit2: New Controller with a little optimization

 

 

public class DupeAcctNumbers { private datetime tminus30 = datetime.now().addDays(-30); private Set<String> acctsLast30 = new Set<String>(); private String AcctNumTmp = ''; private Set<String> dupenums = new Set<String>(); public List<Account> dupes = new List<Account>(); public List<Account> getdupes() { for(Account a30 : [SELECT AccountNumber__c,CreatedDate FROM Account WHERE AccountNumber__c != null AND CreatedDate > :tminus30] ) { acctsLast30.add(a30.AccountNumber__c); } for(Account a : [SELECT AccountNumber__c,Id FROM Account WHERE AccountNumber__c IN :acctsLast30 ORDER BY AccountNumber__c]){ if(a.AccountNumber__c==AcctNumTmp){ dupenums.add(a.AccountNumber__c); AcctNumTmp=a.AccountNumber__c; } else { AcctNumTmp=a.AccountNumber__c; } } for(Account dupe : [SELECT AccountNumber__c,Name,Id,CreatedDate,Createdby.Name,Owner.Name FROM Account WHERE AccountNumber__c in :dupenums ORDER BY AccountNumber__c]) { dupes.add(dupe); } return dupes; } //test method static testMethod void runPositiveTestCases() { Account test1 = new Account(FirstName='Test1',LastName='Test1',AccountNumber__c='12345678'); insert test1; Account test2 = new Account(FirstName='Test2',LastName='Test2',AccountNumber__c='12345678'); insert test2; DupeAcctNumbers dTest = new DupeAcctNumbers(); dTest.getdupes(); } }

 

 

 

Message Edited by David81 on 03-02-2010 01:47 PM
Message Edited by David81 on 03-02-2010 01:56 PM
AvromAvrom

Ah, OK.

 

Well, certainly 500 rows isn't the root of your problem. You should be able to get up to 2000 rows in a request, and put 1000 of them into a list. Do you know about how many rows your second list is returning?

 

I still think you could combine the second and third queries. Instead of adding the account number to dupenums in the first loop, why not just put the account in dupes? You might need to do something slightly tricky to ensure that the *first* account in a chain got in as well, but it wouldn't be very hard. This could make a difference if you're getting up near the 2000 limit.

 

Best,

Avrom

 

David81David81

Alright, so I'm sure this isn't the most elegant way of doing it, but it works. I just changed the query to look for a number a URL parameter to set the number of days to look back and default to 1 if not found.

 

Deploys just fine and works wonderfully in production.

 

 

public class DupeAcctNumbers { private Integer days; private datetime tminus; private Set<String> acctsLast30 = new Set<String>(); private String AcctNumTmp = ''; private Set<String> dupenums = new Set<String>(); public List<Account> dupes = new List<Account>(); public List<Account> getdupes() { if(ApexPages.Currentpage().getParameters().get('days')!=null){ days=integer.valueof(ApexPages.Currentpage().getParameters().get('days')); } else { days = 1; } tminus = datetime.now().addDays(-days); for(Account a30 : [SELECT AccountNumber__c,CreatedDate FROM Account WHERE AccountNumber__c != null AND CreatedDate > :tminus] ) { acctsLast30.add(a30.AccountNumber__c); } for(Account a : [SELECT AccountNumber__c,Id FROM Account WHERE AccountNumber__c IN :acctsLast30 ORDER BY AccountNumber__c]){ if(a.AccountNumber__c==AcctNumTmp){ dupenums.add(a.AccountNumber__c); AcctNumTmp=a.AccountNumber__c; } else { AcctNumTmp=a.AccountNumber__c; } } for(Account dupe : [SELECT AccountNumber__c,Name,Id,CreatedDate,Createdby.Name,Owner.Name FROM Account WHERE AccountNumber__c in :dupenums ORDER BY AccountNumber__c]) { dupes.add(dupe); } return dupes; } //test method static testMethod void runPositiveTestCases() { Account test1 = new Account(FirstName='Test1',LastName='Test1',AccountNumber__c='12345678'); insert test1; Account test2 = new Account(FirstName='Test2',LastName='Test2',AccountNumber__c='12345678'); insert test2; DupeAcctNumbers dTest = new DupeAcctNumbers(); dTest.getdupes(); } }