Sign Up ›

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
Travis Lee 1Travis Lee 1 

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);
  }
}

 
Best Answer chosen by Travis Lee 1
pconpcon
Yes, this pattern is very common and would help you make your tests faster.

As for the comments in your code you would do something like:
  
@isTest 
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Contract> cons = conScoreTestDataFactory.createContractsForVerification(1,0.10,0.05,0.03,0.03,3500,0.10,10000);
    	
        Test.startTest();

        ScoreContract.scoreConTechFee(cons);

        Test.stopTest();
    
        for (Contract contract : cons) {
            System.assertEquals(
                5,
                contract.Score__c,
                'The score should be set to 5'
            );
        }
    }
}

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:
  
@isTest
public class conScoreTestDataFactory {
    public static Account createAccount() {
        Account acc = new Account(
            Name = 'TestAccount',
            Customer_Segment__c = 'Agency',
            CurrencyIsoCode = 'USD',
            Type = 'Prospect'
        );
        insert acc;

        return acc;
    }

    public static Opportunity createOpportunity(Account acc) {
        Opportunity Opp = new Opportunity(
            Name = 'TestOppNew',
            AccountId = acc.Id,
            CloseDate = System.Today().addDays(10),
            StageName = 'Campaign Pitch',
            Type = 'Media',
            Projected_Start_Date__c = System.Today().addDays(15),
            Projected_Annual_Spend__c = 3000000,
            Annual_Commitment__c = 200000
        );
        insert opp;

        return opp;
    }

    public static Contract getContractForVerification(Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Contract c = new Contract (
            AccountId = Acc.Id,
            Opportunity__c = Opp.Id,
            Status = 'Draft',
            Fee_Commitment_Type__c = 'Minimum Fee Commitment',
            ContractTerm = 12,
            Campaign_Technology_Fee__c = campaignTechFee,
            Score__c = 0,
            Platinum_Account_Services_Fee__c = platAccountServiceFee,
            Margin_Management_Module_Fee__c = marginManagementModuleFee,
            Data_Access_and_Integration_Fee__c = dataAccessIntegrationFee,
            Certification_and_Training_Programs__c = certAndTrainingPrograms,
            Campaign_Management_Service_Fee__c = campaignManagementServiceFee,
            Account_Set_Up_Fee__c = acctSetUpFee
        );

        return c;
    }

    public static List<Contract> createContractsForVerification(Integer numCons, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Account acc = createAccount();
        Opportunity opp = createOpportunity(acc);

        List<Contract> cons = new List<Contract>();
        for (Integer i=0; i < numCons; i++) {
            cons.add(getContractForVerification(campaignTechFee, platAccountServiceFee, marginManagementModuleFee, dataAccessIntegrationFee, certAndTrainingPrograms, campaignManagementServiceFee, acctSetUpFee));
        }

        insert cons;

        return cons;
    }
}

Then you can do the following in your test instead
  
@isTest
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Integer> expectedScores = new List<Integer> {
            35,
            30,
            25,
            20,
            15,
            10,
            5
        };

        List<Contract> contracts = new List<Contract>
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.25,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.19,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.17,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.16,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.14,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.12,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.10,0.05,0.03,0.03,3500,0.10,10000));

        Test.startTest();

        ScoreContract.scoreConTechFee(cons);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
}

 

All Answers

pconpcon
Your ContractScore class doesn't have too much room for improvement.  You might be able to do some stuff to make it a little more generic but then you trade off on having it unreadable / unmaintainable.  So I wouldn't worry about that.  Where you have the most room for improvement is in your test.  To start with you should amost never have seeAllData on your tests.  If you don't know why you have it there then you shouldn't have it there.  Secondly, your test is doing too much.  You should have a test method test one and only one thing.  Take a look over this [1] article and you can see how to better create your tests.  Because you have so many if statements you're going to have a lot of tests but you'll have 1) better code coverage 2) better sleep because you know it's working as planned.  Thirdly, you need to add Test.startTest/Test.stopTest around the code you are testing so that you make sure you are specifically testing that.  Lastly, you need to add System.assert* calls to your test.  This will make sure that you get what you expect to get back.  Otherwise you are testing just to get coverage and not testing to make sure your code works as expected.  I would recommend reading over these [2] [3] [4] other resources for your tests as well.

[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/
pconpcon
Also, when adding code please use the "Add a code sample" button (icon <>) to increase readability and make it easier to reference.
Travis Lee 1Travis Lee 1
Hey Patrick,

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?
 
pconpcon
Since you already have your code split out into methods you would do something like
  • 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.
Travis Lee 1Travis Lee 1
scoreConTechFee(cons);
 scoreConMinFeeCommit(cons);
 scoreConProjectedAnnualSpend(cons);
 
scoreConPlatServFee(cons);
scoreConMarginMgmtFee(cons);
scoreConDataAccessFee(cons);
scoreConCertTraining(cons);
scoreConCampaignMgmtFee(cons);
scoreConAcctSetupFee(cons);

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?
pconpcon
Yes, what you have is how I would approach the methods that have more than the two outcomes.
Travis Lee 1Travis Lee 1
Ok, I'll take a crack at it and see if I can come up with a suitable solution, thanks!
Travis Lee 1Travis Lee 1
Hey Patrick,

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
  
@isTest
public class conScoreTestDataFactory {
	public static List<Contract> createContractsForVerification(Integer numCons, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
	
    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;
	List<Contract> cons = new List<Contract>();
		for(Integer i=0;i<numCons;i++) {
			Contract c = new Contract (AccountId = Acc.Id,
					Opportunity__c = Opp.Id,
					Status = 'Draft',
					Fee_Commitment_Type__c = 'Minimum Fee Commitment',
					ContractTerm = 12,
					Campaign_Technology_Fee__c = campaignTechFee,
					Score__c = 0,
					Platinum_Account_Services_Fee__c = platAccountServiceFee,
					Margin_Management_Module_Fee__c = marginManagementModuleFee,
					Data_Access_and_Integration_Fee__c = dataAccessIntegrationFee,
					Certification_and_Training_Programs__c = certAndTrainingPrograms,
					Campaign_Management_Service_Fee__c = campaignManagementServiceFee,
					Account_Set_Up_Fee__c = acctSetUpFee);
			cons.add(c);
		}
		insert cons;
        
        return cons;
	}
}
  
@isTest 
private class conScoreTest {
    
    @isTest static void TestCreateContractTechFeeMin() {
        Contract[] cons = conScoreTestDataFactory.createContractsForVerification(1,0.10,0.05,0.03,0.03,3500,0.10,10000);
    	
        Test.startTest();
        //need to figure this out
        Test.stopTest();
    
        System.assert //need to figure this out too
    }

//then repeat for each method scenario?
}

 
pconpcon
Yes, this pattern is very common and would help you make your tests faster.

As for the comments in your code you would do something like:
  
@isTest 
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Contract> cons = conScoreTestDataFactory.createContractsForVerification(1,0.10,0.05,0.03,0.03,3500,0.10,10000);
    	
        Test.startTest();

        ScoreContract.scoreConTechFee(cons);

        Test.stopTest();
    
        for (Contract contract : cons) {
            System.assertEquals(
                5,
                contract.Score__c,
                'The score should be set to 5'
            );
        }
    }
}

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:
  
@isTest
public class conScoreTestDataFactory {
    public static Account createAccount() {
        Account acc = new Account(
            Name = 'TestAccount',
            Customer_Segment__c = 'Agency',
            CurrencyIsoCode = 'USD',
            Type = 'Prospect'
        );
        insert acc;

        return acc;
    }

    public static Opportunity createOpportunity(Account acc) {
        Opportunity Opp = new Opportunity(
            Name = 'TestOppNew',
            AccountId = acc.Id,
            CloseDate = System.Today().addDays(10),
            StageName = 'Campaign Pitch',
            Type = 'Media',
            Projected_Start_Date__c = System.Today().addDays(15),
            Projected_Annual_Spend__c = 3000000,
            Annual_Commitment__c = 200000
        );
        insert opp;

        return opp;
    }

    public static Contract getContractForVerification(Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Contract c = new Contract (
            AccountId = Acc.Id,
            Opportunity__c = Opp.Id,
            Status = 'Draft',
            Fee_Commitment_Type__c = 'Minimum Fee Commitment',
            ContractTerm = 12,
            Campaign_Technology_Fee__c = campaignTechFee,
            Score__c = 0,
            Platinum_Account_Services_Fee__c = platAccountServiceFee,
            Margin_Management_Module_Fee__c = marginManagementModuleFee,
            Data_Access_and_Integration_Fee__c = dataAccessIntegrationFee,
            Certification_and_Training_Programs__c = certAndTrainingPrograms,
            Campaign_Management_Service_Fee__c = campaignManagementServiceFee,
            Account_Set_Up_Fee__c = acctSetUpFee
        );

        return c;
    }

    public static List<Contract> createContractsForVerification(Integer numCons, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Account acc = createAccount();
        Opportunity opp = createOpportunity(acc);

        List<Contract> cons = new List<Contract>();
        for (Integer i=0; i < numCons; i++) {
            cons.add(getContractForVerification(campaignTechFee, platAccountServiceFee, marginManagementModuleFee, dataAccessIntegrationFee, certAndTrainingPrograms, campaignManagementServiceFee, acctSetUpFee));
        }

        insert cons;

        return cons;
    }
}

Then you can do the following in your test instead
  
@isTest
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Integer> expectedScores = new List<Integer> {
            35,
            30,
            25,
            20,
            15,
            10,
            5
        };

        List<Contract> contracts = new List<Contract>
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.25,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.19,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.17,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.16,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.14,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.12,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.10,0.05,0.03,0.03,3500,0.10,10000));

        Test.startTest();

        ScoreContract.scoreConTechFee(cons);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
}

 
This was selected as the best answer
Travis Lee 1Travis Lee 1
This is awesome! Thanks! I'm glad that theory worked out, I was actually just working through this module in trailhead so it's nice to be able to apply something that isn't just theory. Any ideas now why the developer console would be showing a "variable does not exist" error for ScoreContract? (line 9 in your first code block). It's in the main class so it should be fine right?
Travis Lee 1Travis Lee 1
I get the same error for line 33 of the 2nd code block, but not on line 18? Same variable? 
pconpcon
Whoops.  Yeah your class is named ContractScore not ScoreContract.
Travis Lee 1Travis Lee 1
Oh ok, I thought it was aimed at calling the scoreContract method within the class. Why would it be flagging contracts.add as an unexpected token?
pconpcon
Because line 15 of the test should read:
  
List<Contract> contracts = new List<Contract>();
Travis Lee 1Travis Lee 1
@isTest
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Integer> expectedScores = new List<Integer> {35,30,25,20,15,10,5};

            List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.25,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.19,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.17,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.16,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.14,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.12,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.10,0.05,0.03,0.03,3500,0.10,10000));

        Test.startTest();

        ContractScore.scoreConTechFee(cons);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
}
Line 18 of conScoreTest says that the 'cons' variable doesn't exist? 

  
public static Contract getContractForVerification(Account acc, Opportunity opp, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Contract c = new Contract (
            AccountId = acc.Id,
            Opportunity__c = opp.Id,
            Status = 'Draft',
            Fee_Commitment_Type__c = 'Minimum Fee Commitment',
            ContractTerm = 12,
            Campaign_Technology_Fee__c = campaignTechFee,
            Score__c = 0,
            Platinum_Account_Services_Fee__c = platAccountServiceFee,
            Margin_Management_Module_Fee__c = marginManagementModuleFee,
            Data_Access_and_Integration_Fee__c = dataAccessIntegrationFee,
            Certification_and_Training_Programs__c = certAndTrainingPrograms,
            Campaign_Management_Service_Fee__c = campaignManagementServiceFee,
            Account_Set_Up_Fee__c = acctSetUpFee
        );

        return c;
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?
 
pconpcon
Line 18 says that because I renamed the variable to contracts instead of cons.  Line 18 should read
  
ContractScore.scoreConTechFee(contracts);

And you are right about the data factory
  
@isTest
public class conScoreTestDataFactory {
    public static Account createAccount() {
        Account acc = new Account(
            Name = 'TestAccount',
            Customer_Segment__c = 'Agency',
            CurrencyIsoCode = 'USD',
            Type = 'Prospect'
        );
        insert acc;

        return acc;
    }

    public static Opportunity createOpportunity(Account acc) {
        Opportunity Opp = new Opportunity(
            Name = 'TestOppNew',
            AccountId = acc.Id,
            CloseDate = System.Today().addDays(10),
            StageName = 'Campaign Pitch',
            Type = 'Media',
            Projected_Start_Date__c = System.Today().addDays(15),
            Projected_Annual_Spend__c = 3000000,
            Annual_Commitment__c = 200000
        );
        insert opp;

        return opp;
    }

    public static Contract getContractForVerification(Account acc, Opportunity opp, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Contract c = new Contract (
            AccountId = acc.Id,
            Opportunity__c = opp.Id,
            Status = 'Draft',
            Fee_Commitment_Type__c = 'Minimum Fee Commitment',
            ContractTerm = 12,
            Campaign_Technology_Fee__c = campaignTechFee,
            Score__c = 0,
            Platinum_Account_Services_Fee__c = platAccountServiceFee,
            Margin_Management_Module_Fee__c = marginManagementModuleFee,
            Data_Access_and_Integration_Fee__c = dataAccessIntegrationFee,
            Certification_and_Training_Programs__c = certAndTrainingPrograms,
            Campaign_Management_Service_Fee__c = campaignManagementServiceFee,
            Account_Set_Up_Fee__c = acctSetUpFee
        );

        return c;
    }

    public static List<Contract> createContractsForVerification(Integer numCons, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Account acc = createAccount();
        Opportunity opp = createOpportunity(acc);

        List<Contract> cons = new List<Contract>();
        for (Integer i = 0; i < numCons; i++) {
            cons.add(getContractForVerification(acc, opp, campaignTechFee, platAccountServiceFee, marginManagementModuleFee, dataAccessIntegrationFee, certAndTrainingPrograms, campaignManagementServiceFee, acctSetUpFee));
        }

        insert cons;

        return cons;
    }
}

 
Travis Lee 1Travis Lee 1
So with that change, that'll mean I need to add an account and opp creation on scoreTest too? Since the method won't be able to access the acc and opp variables to use them on lines 8-14? 
@isTest
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Integer> expectedScores = new List<Integer> {35,30,25,20,15,10,5};

            List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.25,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.19,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.17,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.16,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.14,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.12,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(0.10,0.05,0.03,0.03,3500,0.10,10000));

        Test.startTest();

        ContractScore.scoreConTechFee(cons);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
}
pconpcon
You test would be updated to
  
@isTest
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Integer> expectedScores = new List<Integer> {35,30,25,20,15,10,5};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(testAccount);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.25,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.19,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.17,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.16,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.14,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.12,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,3500,0.10,10000));

        Test.startTest();

        ContractScore.scoreConTechFee(cons);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
}

 
Travis Lee 1Travis Lee 1
Hey Patrick, I've built it out to his point but I'm stuck on two of the test methods (TestScoreConMinFeeCommit, and TestScoreConProjectAnnualSpend) because those are formula fields that are pulled from the Opportunity and I can't adjust the fields unless I access the opp, right? I started updating the data factory to deal with thoes parameters but I'm not sure if my actions will have any negative effects downstream. Thoughts?

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?  
@isTest
public class conScoreTestDataFactory {
    public static Account createAccount() {
        Account acc = new Account(
            Name = 'TestAccount',
            Customer_Segment__c = 'Agency',
            CurrencyIsoCode = 'USD',
            Type = 'Prospect'
        );
        insert acc;

        return acc;
    }

    public static Opportunity createOpportunity(Account acc, Integer projAnnualSpend, Integer minFeeCommit) {
        Opportunity Opp = new Opportunity(
            Name = 'TestOppNew',
            AccountId = acc.Id,
            CloseDate = System.Today().addDays(10),
            StageName = 'Campaign Pitch',
            Type = 'Media',
            Projected_Start_Date__c = System.Today().addDays(15),
            Projected_Annual_Spend__c = projAnnualSpend,
            Annual_Commitment__c = minFeeCommit
        );
        insert opp;

        return opp;
    }

    public static Contract getContractForVerification(Account acc, Opportunity opp, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Contract c = new Contract (
            AccountId = acc.Id,
            Opportunity__c = opp.Id,
            Status = 'Draft',
            Fee_Commitment_Type__c = 'Minimum Fee Commitment',
            ContractTerm = 12,
            Campaign_Technology_Fee__c = campaignTechFee,
            Score__c = 0,
            Platinum_Account_Services_Fee__c = platAccountServiceFee,
            Margin_Management_Module_Fee__c = marginManagementModuleFee,
            Data_Access_and_Integration_Fee__c = dataAccessIntegrationFee,
            Certification_and_Training_Programs__c = certAndTrainingPrograms,
            Campaign_Management_Service_Fee__c = campaignManagementServiceFee,
            Account_Set_Up_Fee__c = acctSetUpFee
        );

        return c;
    }

    public static List<Contract> createContractsForVerification(Integer numCons, Decimal campaignTechFee, Decimal platAccountServiceFee, Decimal marginManagementModuleFee, Decimal dataAccessIntegrationFee, Integer certAndTrainingPrograms, Decimal campaignManagementServiceFee, Integer acctSetUpFee) {
        Account acc = createAccount();
        Opportunity opp = createOpportunity(acc, projAnnualSpend, minFeeCommit);

        List<Contract> cons = new List<Contract>();
        for (Integer i=0; i < numCons; i++) {
            cons.add(getContractForVerification(acc, opp, campaignTechFee, platAccountServiceFee, marginManagementModuleFee, dataAccessIntegrationFee, certAndTrainingPrograms, campaignManagementServiceFee, acctSetUpFee));
        }

        insert cons;

        return cons;
    }
}



  
@isTest
private class conScoreTest {
    @isTest
    static void TestScoreConTechFee() {
        List<Integer> expectedScores = new List<Integer> {80,60,55,50,45,40,35};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.25,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.19,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.17,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.16,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.14,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.12,0.05,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,3500,0.10,10000));

        Test.startTest();

        ContractScore.scoreConTechFee(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	
    @isTest
    static void TestScoreConMinFeeCommit() {
        List<Integer> expectedScores = new List<Integer> {80,60,55,50,45,40,35};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.25,0.05,0.03,0.03,3500,0.10,10000));
        
        Test.startTest();

        ContractScore.scoreConMinFeeCommit(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
    
    @isTest
    static void TestScoreConProjectedAnnualSpend() {
        List<Integer> expectedScores = new List<Integer> {80,60,55,50,45,40,35};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.25,0.05,0.03,0.03,3500,0.10,10000));
        
        Test.startTest();

        ContractScore.scoreConProjectedAnnualSpend(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
    
	@isTest
    static void TestScoreConPlatServFee() {
        List<Integer> expectedScores = new List<Integer> {35,30};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.07,0.03,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.03,0.03,0.03,3500,0.10,10000));
        

        Test.startTest();

        ContractScore.scoreConPlatServFee(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	
	@isTest
    static void TestScoreConMarginMgmtFee() {
        List<Integer> expectedScores = new List<Integer> {35,30};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.05,0.03,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.01,0.03,3500,0.10,10000));
        

        Test.startTest();

        ContractScore.scoreConMarginMgmtFee(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	
	@isTest
    static void TestScoreConDataAccessFee() {
        List<Integer> expectedScores = new List<Integer> {35,30};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.05,3500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.01,3500,0.10,10000));
        

        Test.startTest();

        ContractScore.scoreConDataAccessFee(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	
	@isTest
    static void TestScoreConCertTraining() {
        List<Integer> expectedScores = new List<Integer> {35,30};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,4500,0.10,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,2500,0.10,10000));
        

        Test.startTest();

        ContractScore.scoreConCertTraining(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	
	@isTest
    static void TestScoreConCampaignMgmtFee() {
        List<Integer> expectedScores = new List<Integer> {35,30};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,3500,0.12,10000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,3500,0.08,10000));
        

        Test.startTest();

        ContractScore.scoreConCampaignMgmtFee(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	
	@isTest
    static void TestScoreConAcctSetupFee() {
        List<Integer> expectedScores = new List<Integer> {35,30};

        Account acc = conScoreTestDataFactory.createAccount();
        Opportunity opp = conScoreTestDataFactory.createOpportunity(acc);

        List<Contract> contracts = new List<Contract> ();
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,3500,0.10,12000));
        contracts.add(conScoreTestDataFactory.getContractForVerification(acc, opp, 0.10,0.05,0.03,0.03,3500,0.10,8000));
        

        Test.startTest();

        ContractScore.scoreConAcctSetupFee(contracts);

        Test.stopTest();

        for (Integer i = 0; i < contracts.size(); i++) {
            System.assertEquals(
                expectedScores.get(i),
                contracts.get(i).Score__c,
                'Did not get the expected score back'
            );
        }
    }
	//add more test methods here
}