You need to sign in to do that
Don't have an account?
Could this code be improved?
Hey everyone, I'm a new developer trying to understand the intricacies of apex code to be a bit more self sufficient. I'm still a bit hazy on proper test classes so what you'll see below are the class that I'm using to compile a score for contracts and the test class for it. The test class currently has 81% coverage so it will deploy but I have the feeling it could be done more efficiently? Any suggestions are welcome! It's a little lengthy...
public class ContractScore
{
// Methods for scoring Contracts..
public static void scoreContract(List<Contract> cons)
{
// Entry point, runs all scoring methods on list of Contracts
// Handle existing contracts that may have a null value in the Score__c field
for(Contract con : cons)
{
con.Score__c = 0;
}
// Call individual scoring methods
scoreConTechFee(cons);
scoreConMinFeeCommit(cons);
scoreConProjectedAnnualSpend(cons);
scoreConPlatServFee(cons);
scoreConMarginMgmtFee(cons);
scoreConDataAccessFee(cons);
scoreConCertTraining(cons);
scoreConCampaignMgmtFee(cons);
scoreConAcctSetupFee(cons);
// etc..
// Update contract scores
update cons;
}
public static void scoreConTechFee(List<Contract> cons)
{
// Score Contracts based on Campaign Tech Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Campaign_Technology_Fee__c >= 0.20){
con.Score__c += 35;
} else if (con.Campaign_Technology_Fee__c < 0.20 && con.Campaign_Technology_Fee__c >= 0.18 ) {
con.Score__c += 30;
} else if (con.Campaign_Technology_Fee__c < 0.18 && con.Campaign_Technology_Fee__c >= 0.17) {
con.Score__c += 25;
} else if (con.Campaign_Technology_Fee__c < 0.17 && con.Campaign_Technology_Fee__c >= 0.15) {
con.Score__c += 20;
} else if (con.Campaign_Technology_Fee__c < 0.15 && con.Campaign_Technology_Fee__c >= 0.13) {
con.Score__c += 15;
} else if (con.Campaign_Technology_Fee__c < 0.13 && con.Campaign_Technology_Fee__c >= 0.11) {
con.Score__c += 10;
} else if (con.Campaign_Technology_Fee__c < 0.11) {
con.Score__c += 5;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConMinFeeCommit(List<Contract> cons)
{
// Score Contracts based on Minimum Annual Fee Commitment (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Minimum_Annual_Fee_Commitment__c < 100000){
con.Score__c += 0;
} else if (con.Minimum_Annual_Fee_Commitment__c >= 100000 && con.Minimum_Annual_Fee_Commitment__c <= 200000) {
con.Score__c += 10;
} else if (con.Minimum_Annual_Fee_Commitment__c > 200000 && con.Minimum_Annual_Fee_Commitment__c <= 400000){
con.Score__c += 15;
} else if (con.Minimum_Annual_Fee_Commitment__c > 400000 && con.Minimum_Annual_Fee_Commitment__c <= 600000){
con.Score__c += 20;
} else if (con.Minimum_Annual_Fee_Commitment__c > 600000 && con.Minimum_Annual_Fee_Commitment__c <= 800000){
con.Score__c += 25;
} else if (con.Minimum_Annual_Fee_Commitment__c > 800000 && con.Minimum_Annual_Fee_Commitment__c <= 1000000){
con.Score__c += 30;
} else if (con.Minimum_Annual_Fee_Commitment__c > 1000000){
con.Score__c += 50;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConProjectedAnnualSpend(List<Contract> cons)
{
// Score Contracts based on Projected Annual Spend (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Projected_Annual_Spend__c < 2000000){
con.Score__c += 0;
} else if (con.Projected_Annual_Spend__c >= 2000000 && con.Projected_Annual_Spend__c <= 4000000) {
con.Score__c += 10;
} else if (con.Projected_Annual_Spend__c > 4000000 && con.Projected_Annual_Spend__c <= 6000000){
con.Score__c += 15;
} else if (con.Projected_Annual_Spend__c > 6000000 && con.Projected_Annual_Spend__c <= 8000000){
con.Score__c += 20;
} else if (con.Projected_Annual_Spend__c > 8000000 && con.Projected_Annual_Spend__c <= 10000000){
con.Score__c += 30;
} else if (con.Projected_Annual_Spend__c > 10000000){
con.Score__c += 50;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConPlatServFee(List<Contract> cons)
{
// Score Contracts based on Platinum Account Services Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Platinum_Account_Services_Fee__c < 0.05){
con.Score__c -= 5;
} else if (con.Platinum_Account_Services_Fee__c >= 0.05 ) {
con.Score__c += 5;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConMarginMgmtFee(List<Contract> cons)
{
// Score Contracts based on Margin Management Module Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Margin_Management_Module_Fee__c < 0.03){
con.Score__c -= 5;
} else if (con.Margin_Management_Module_Fee__c >= 0.03) {
con.Score__c += 5;
}
}
}
public static void scoreConDataAccessFee(List<Contract> cons)
{
// Score Contracts based on Data Access and Integration Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Data_Access_and_Integration_Fee__c < 0.03){
con.Score__c -= 5;
} else if (con.Data_Access_and_Integration_Fee__c >= 0.03) {
con.Score__c += 5;
}
}
}
public static void scoreConCertTraining(List<Contract> cons)
{
// Score Contracts based on Certification and Training Programs (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Certification_and_Training_Programs__c < 3500){
con.Score__c -= 5;
} else if (con.Certification_and_Training_Programs__c >= 3500) {
con.Score__c += 5;
}
}
}
public static void scoreConCampaignMgmtFee(List<Contract> cons)
{
// Score Contracts based on Campaign Management Service Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Campaign_Management_Service_Fee__c < 0.10){
con.Score__c -= 5;
} else if (con.Campaign_Management_Service_Fee__c >= 0.10) {
con.Score__c += 5;
}
}
}
public static void scoreConAcctSetupFee(List<Contract> cons)
{
// Score Contracts based on Account Set Up Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Account_Set_Up_Fee__c < 10000){
con.Score__c -= 5;
} else if (con.Account_Set_Up_Fee__c >= 10000) {
con.Score__c += 5;
}
}
}
// Other methods here
}
--------------------------------------------------------------------------------
@isTest (SeeAllData = true)
public class conScoreTest {
public static testMethod void validateConScoreTest(){
Account Acc = new Account();
Acc.Name = 'TestAccount';
Acc.Customer_Segment__c = 'Agency';
Acc.CurrencyIsoCode = 'USD';
Acc.Type = 'Prospect';
Insert Acc;
Opportunity Opp = new Opportunity();
Opp.Name = 'TestOppNew';
Opp.AccountId = Acc.Id;
Opp.CloseDate = System.Today().addDays(10);
Opp.StageName = 'Campaign Pitch';
Opp.Type = 'Media';
Opp.Projected_Start_Date__c = System.Today().addDays(15);
Opp.Projected_Annual_Spend__c = 3000000;
Opp.Annual_Commitment__c = 200000;
Insert Opp;
Contract Con = new Contract();
Con.AccountId = Acc.Id;
Con.Opportunity__c = Opp.Id;
Con.Status = 'Draft';
Con.Fee_Commitment_Type__c = 'Minimum Fee Commitment';
Con.ContractTerm = 12;
Con.Campaign_Technology_Fee__c = 0.18;
Con.Score__c = 0;
Con.Platinum_Account_Services_Fee__c = 0.05;
Con.Margin_Management_Module_Fee__c = 0.03;
Con.Data_Access_and_Integration_Fee__c = 0.03;
Con.Certification_and_Training_Programs__c = 3500;
Con.Campaign_Management_Service_Fee__c = 0.10;
Con.Account_Set_Up_Fee__c = 10000;
Insert Con;
Con.Campaign_Technology_Fee__c = 0.20;
Con.Platinum_Account_Services_Fee__c = 0.07;
Con.Margin_Management_Module_Fee__c = 0.05;
Con.Data_Access_and_Integration_Fee__c = 0.05;
Con.Certification_and_Training_Programs__c = 5000;
Con.Campaign_Management_Service_Fee__c = 0.12;
Con.Account_Set_Up_Fee__c = 20000;
Update Con;
Con.Campaign_Technology_Fee__c = 0.16;
Con.Platinum_Account_Services_Fee__c = 0.04;
Con.Margin_Management_Module_Fee__c = 0.02;
Con.Data_Access_and_Integration_Fee__c = 0.02;
Con.Certification_and_Training_Programs__c = 2000;
Con.Campaign_Management_Service_Fee__c = 0.09;
Con.Account_Set_Up_Fee__c = 9000;
Update Con;
// tjl - test method
List<Contract> conList = new List<Contract>();
conList.add(Con);
ContractScore.scoreContract(conList);
}
}
public class ContractScore
{
// Methods for scoring Contracts..
public static void scoreContract(List<Contract> cons)
{
// Entry point, runs all scoring methods on list of Contracts
// Handle existing contracts that may have a null value in the Score__c field
for(Contract con : cons)
{
con.Score__c = 0;
}
// Call individual scoring methods
scoreConTechFee(cons);
scoreConMinFeeCommit(cons);
scoreConProjectedAnnualSpend(cons);
scoreConPlatServFee(cons);
scoreConMarginMgmtFee(cons);
scoreConDataAccessFee(cons);
scoreConCertTraining(cons);
scoreConCampaignMgmtFee(cons);
scoreConAcctSetupFee(cons);
// etc..
// Update contract scores
update cons;
}
public static void scoreConTechFee(List<Contract> cons)
{
// Score Contracts based on Campaign Tech Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Campaign_Technology_Fee__c >= 0.20){
con.Score__c += 35;
} else if (con.Campaign_Technology_Fee__c < 0.20 && con.Campaign_Technology_Fee__c >= 0.18 ) {
con.Score__c += 30;
} else if (con.Campaign_Technology_Fee__c < 0.18 && con.Campaign_Technology_Fee__c >= 0.17) {
con.Score__c += 25;
} else if (con.Campaign_Technology_Fee__c < 0.17 && con.Campaign_Technology_Fee__c >= 0.15) {
con.Score__c += 20;
} else if (con.Campaign_Technology_Fee__c < 0.15 && con.Campaign_Technology_Fee__c >= 0.13) {
con.Score__c += 15;
} else if (con.Campaign_Technology_Fee__c < 0.13 && con.Campaign_Technology_Fee__c >= 0.11) {
con.Score__c += 10;
} else if (con.Campaign_Technology_Fee__c < 0.11) {
con.Score__c += 5;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConMinFeeCommit(List<Contract> cons)
{
// Score Contracts based on Minimum Annual Fee Commitment (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Minimum_Annual_Fee_Commitment__c < 100000){
con.Score__c += 0;
} else if (con.Minimum_Annual_Fee_Commitment__c >= 100000 && con.Minimum_Annual_Fee_Commitment__c <= 200000) {
con.Score__c += 10;
} else if (con.Minimum_Annual_Fee_Commitment__c > 200000 && con.Minimum_Annual_Fee_Commitment__c <= 400000){
con.Score__c += 15;
} else if (con.Minimum_Annual_Fee_Commitment__c > 400000 && con.Minimum_Annual_Fee_Commitment__c <= 600000){
con.Score__c += 20;
} else if (con.Minimum_Annual_Fee_Commitment__c > 600000 && con.Minimum_Annual_Fee_Commitment__c <= 800000){
con.Score__c += 25;
} else if (con.Minimum_Annual_Fee_Commitment__c > 800000 && con.Minimum_Annual_Fee_Commitment__c <= 1000000){
con.Score__c += 30;
} else if (con.Minimum_Annual_Fee_Commitment__c > 1000000){
con.Score__c += 50;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConProjectedAnnualSpend(List<Contract> cons)
{
// Score Contracts based on Projected Annual Spend (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Projected_Annual_Spend__c < 2000000){
con.Score__c += 0;
} else if (con.Projected_Annual_Spend__c >= 2000000 && con.Projected_Annual_Spend__c <= 4000000) {
con.Score__c += 10;
} else if (con.Projected_Annual_Spend__c > 4000000 && con.Projected_Annual_Spend__c <= 6000000){
con.Score__c += 15;
} else if (con.Projected_Annual_Spend__c > 6000000 && con.Projected_Annual_Spend__c <= 8000000){
con.Score__c += 20;
} else if (con.Projected_Annual_Spend__c > 8000000 && con.Projected_Annual_Spend__c <= 10000000){
con.Score__c += 30;
} else if (con.Projected_Annual_Spend__c > 10000000){
con.Score__c += 50;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConPlatServFee(List<Contract> cons)
{
// Score Contracts based on Platinum Account Services Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Platinum_Account_Services_Fee__c < 0.05){
con.Score__c -= 5;
} else if (con.Platinum_Account_Services_Fee__c >= 0.05 ) {
con.Score__c += 5;
} else {
con.Score__c += 0;
}
}
}
public static void scoreConMarginMgmtFee(List<Contract> cons)
{
// Score Contracts based on Margin Management Module Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Margin_Management_Module_Fee__c < 0.03){
con.Score__c -= 5;
} else if (con.Margin_Management_Module_Fee__c >= 0.03) {
con.Score__c += 5;
}
}
}
public static void scoreConDataAccessFee(List<Contract> cons)
{
// Score Contracts based on Data Access and Integration Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Data_Access_and_Integration_Fee__c < 0.03){
con.Score__c -= 5;
} else if (con.Data_Access_and_Integration_Fee__c >= 0.03) {
con.Score__c += 5;
}
}
}
public static void scoreConCertTraining(List<Contract> cons)
{
// Score Contracts based on Certification and Training Programs (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Certification_and_Training_Programs__c < 3500){
con.Score__c -= 5;
} else if (con.Certification_and_Training_Programs__c >= 3500) {
con.Score__c += 5;
}
}
}
public static void scoreConCampaignMgmtFee(List<Contract> cons)
{
// Score Contracts based on Campaign Management Service Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Campaign_Management_Service_Fee__c < 0.10){
con.Score__c -= 5;
} else if (con.Campaign_Management_Service_Fee__c >= 0.10) {
con.Score__c += 5;
}
}
}
public static void scoreConAcctSetupFee(List<Contract> cons)
{
// Score Contracts based on Account Set Up Fee (Contract object)
// Loop through cons and add score
for(Contract con : cons)
{
// Find proper picklist option and add value to score
if(con.Account_Set_Up_Fee__c < 10000){
con.Score__c -= 5;
} else if (con.Account_Set_Up_Fee__c >= 10000) {
con.Score__c += 5;
}
}
}
// Other methods here
}
--------------------------------------------------------------------------------
@isTest (SeeAllData = true)
public class conScoreTest {
public static testMethod void validateConScoreTest(){
Account Acc = new Account();
Acc.Name = 'TestAccount';
Acc.Customer_Segment__c = 'Agency';
Acc.CurrencyIsoCode = 'USD';
Acc.Type = 'Prospect';
Insert Acc;
Opportunity Opp = new Opportunity();
Opp.Name = 'TestOppNew';
Opp.AccountId = Acc.Id;
Opp.CloseDate = System.Today().addDays(10);
Opp.StageName = 'Campaign Pitch';
Opp.Type = 'Media';
Opp.Projected_Start_Date__c = System.Today().addDays(15);
Opp.Projected_Annual_Spend__c = 3000000;
Opp.Annual_Commitment__c = 200000;
Insert Opp;
Contract Con = new Contract();
Con.AccountId = Acc.Id;
Con.Opportunity__c = Opp.Id;
Con.Status = 'Draft';
Con.Fee_Commitment_Type__c = 'Minimum Fee Commitment';
Con.ContractTerm = 12;
Con.Campaign_Technology_Fee__c = 0.18;
Con.Score__c = 0;
Con.Platinum_Account_Services_Fee__c = 0.05;
Con.Margin_Management_Module_Fee__c = 0.03;
Con.Data_Access_and_Integration_Fee__c = 0.03;
Con.Certification_and_Training_Programs__c = 3500;
Con.Campaign_Management_Service_Fee__c = 0.10;
Con.Account_Set_Up_Fee__c = 10000;
Insert Con;
Con.Campaign_Technology_Fee__c = 0.20;
Con.Platinum_Account_Services_Fee__c = 0.07;
Con.Margin_Management_Module_Fee__c = 0.05;
Con.Data_Access_and_Integration_Fee__c = 0.05;
Con.Certification_and_Training_Programs__c = 5000;
Con.Campaign_Management_Service_Fee__c = 0.12;
Con.Account_Set_Up_Fee__c = 20000;
Update Con;
Con.Campaign_Technology_Fee__c = 0.16;
Con.Platinum_Account_Services_Fee__c = 0.04;
Con.Margin_Management_Module_Fee__c = 0.02;
Con.Data_Access_and_Integration_Fee__c = 0.02;
Con.Certification_and_Training_Programs__c = 2000;
Con.Campaign_Management_Service_Fee__c = 0.09;
Con.Account_Set_Up_Fee__c = 9000;
Update Con;
// tjl - test method
List<Contract> conList = new List<Contract>();
conList.add(Con);
ContractScore.scoreContract(conList);
}
}
As for the comments in your code you would do something like:
You may want to make your test data factory to just create a single contract and then you can build out mutliple contracts and test them. For example:
Then you can do the following in your test instead
All Answers
[1] http://blog.deadlypenguin.com/blog/testing/strategies/
[2] http://www.sfdc99.com/2013/05/14/how-to-write-a-test-class/
[3] http://pcon.github.io/presentations/testing/
[4] http://blog.deadlypenguin.com/blog/2014/07/23/intro-to-apex-auto-converting-leads-in-a-trigger/
Read through [1] and [2] and in the middle of [3] currently. Just want to be sure I'm understanding this correctly when it comes to breaking my class into testable parts because it seems like there would be a ridiculous amount? For the one of the first methods scoreMinFeeCommit for example, this is how I would follow your template to hit all the necessary scenarios:
insert single minimum amount (below 100k)
update single minimum amount (below 100k)
insert single between 100k and 200k
update single between 100k and 200k
insert single between 200k and 400k
update single between 400k and 600k
insert single between 600k and 800k
update single between 800k and 1mil
insert single max amount (above 1 mil)
update single max amount (above 1 mil)
insert bulk minimum amount (below 100k)
update bulk minimum amount (below 100k)
insert bulk between 100k and 200k
update bulk between 100k and 200k
insert bulk between 200k and 400k
update bulk between 400k and 600k
insert bulk between 600k and 800k
update bulk between 800k and 1mil
insert bulk max amount (above 1 mil)
update bulk max amount (above 1 mil)
And that's only for 1 of the methods. Is that right?
- scoreConCampaignMgmtFee_lessThan_pointTen
- scoreConCampaignMgmtFee_greaterThan_pointTen
- scoreConDataAccessFee_lessThan_pointOhThree
- scoreConDataAccessFee_greaterThan_pointOhThree
And so forth for all of your possibilities. Then you'd pass in a list of Contracts and verify that the score was modified as expected. Alternatively you could just do one test per method and pass in multiple Contracts that meet all of the criteria and then verify that all of their score changed. And then you'd have a couple of tests that run your scoreContracts method that then runs all of them and verify that the cumlative score changes as expected.So the methods from the last grouping here (platservfee --> acctsetupfee) make sense in that i only have to handle two "scenarios": whether they are above or below the threshold value. But since the first three methods (contechfee, minfeecommit, and conprojectedannualspend) have multiple levels, does my above post make sense in terms of how to attack the scoreConMinFeeCommit it for full coverage?
I was doing some further research, would this data factory idea be of help for creating the test records? (single vs. bulk tests especially?)
https://developer.salesforce.com/trailhead/apex_testing/apex_testing_data I feel like it's pertinent if I had a chunk below this existing code to add the contracts? But how would I make it syntactically correct so that I have the ability to pass in differing amounts to the Contract fields like what this was originally doing with the numAccts variable? Is that even a possibility? Or did I just make this more complicated?
I just saw that Data Factory post as a way to more efficiently create the necessary test records
As for the comments in your code you would do something like:
You may want to make your test data factory to just create a single contract and then you can build out mutliple contracts and test them. For example:
Then you can do the following in your test instead
Also, is adding 'Account acc' and 'Opportunity opp' parameters to overcome that error on the acc.Id? The next error it brings up in the Data Factory is that the getContractForVerification method either does not exist or has an improper signature but it looks right?
And you are right about the data factory
On a side note, without taking the opp fields into consideration, the class is receiving partial code coverage for the lines I've included below but none of the methods are being greenlit?