Skip to content

Feedback #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
github-classroom wants to merge 10 commits into
base: feedback
Choose a base branch
from
Open

Feedback #1

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
04b8e4b
Setting up GitHub Classroom Feedback
github-classroom[bot] Apr 10, 2024
2ee852d
add online IDE url
github-classroom[bot] Apr 10, 2024
c3ee2b6
Solution 1 & 2 commit
arieltahimik Apr 27, 2024
1b29c53
Question 1-4 solution
arieltahimik Apr 28, 2024
e9cadfb
Question 5 solution
arieltahimik Apr 29, 2024
0d34c0e
Question 6 solution
arieltahimik May 1, 2024
d395a08
Question 7 solution
arieltahimik May 5, 2024
21ba6b9
Question 7 solution
arieltahimik May 5, 2024
6c6c3f7
AccountTrigger refactored
arieltahimik Dec 6, 2024
fee9215
OpportunityTrigger updated
arieltahimik Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[![Open in Visual Studio Code](https://classroom.github.com/assets/open-in-vscode-718a45dd9cf7e7f842a935f5ebbe5719a5e09af4491e668f4dbf3b35d5cca122.svg)](https://classroom.github.com/online_ide?assignment_repo_id=14674892&assignment_repo_type=AssignmentRepo)

# Developer Kickstart Module 6: Apex Triggers

Expand Down
42 changes: 42 additions & 0 deletions force-app/main/default/triggers/AccountTrigger.trigger
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
trigger AccountTrigger on Account (before insert, after insert) {

switch on Trigger.operationType {
when BEFORE_INSERT {
for (Account acc : Trigger.new) {
//Solution 1
if (acc.Type == null) {
acc.Type = 'Prospect';
}

//Solution 2
acc.BillingStreet = acc.ShippingStreet ?? '';
acc.BillingCity = acc.ShippingCity ?? '';
acc.BillingState = acc.ShippingState ?? '';
acc.BillingPostalCode = acc.ShippingPostalCode ?? '';
acc.BillingCountry = acc.ShippingCountry ?? '';

//Solution 3
if (acc.Fax != null && acc.Phone != null && acc.Website != null) {
acc.Rating = 'Hot';
}
}
}
when AFTER_INSERT {
//Solution 4
List<Contact> contactsToInsert = new List<Contact>();
for (Account acc : Trigger.new) {
Contact con = new Contact();
con.LastName = 'DefaultContact';
con.Email = 'default@email.com';
con.AccountId = acc.Id;
contactsToInsert.add(con);
}
if (contactsToInsert.size() > 0) {
insert contactsToInsert;
}
}
when else {
System.debug('AccountTrigger WHEN ELSE ACTIVATED');

Check warning

Code scanning / PMD

Debug statements contribute to longer transactions and consume Apex CPU time even when debug logs are not being captured. When possible make use of other debugging techniques such as the Apex Replay Debugger and Checkpoints that could cover *most* use cases. For other valid use cases that the statement is in fact valid make use of the `@SuppressWarnings` annotation or the `//NOPMD` comment. Warning

Avoid debug statements since they impact on performance

Check warning

Code scanning / PMD

The first parameter of System.debug, when using the signature with two parameters, is a LoggingLevel enum. Having the Logging Level specified provides a cleaner log, and improves readability of it. Warning

Calls to System.debug should specify a logging level.
}
}
Copy link

@bdJohnson72 bdJohnson72 May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really good! I typically hate seeing the curly braces with the logic on the same line as a predicate condition. But in this case, I think it made your mode much more readable. I like it.

Another thing you could explore is the use of the null coalescing operator to assign a default value if something evaluates to null

 private void setDefaultBillingAddress(){
        for(Account account : accounts){
            account.BillingStreet = account.ShippingStreet ?? '';
            account.BillingCity = account.ShippingCity ?? '';
            account.BillingState = account.ShippingState ?? '';
            account.BillingPostalCode = account.ShippingPostalCode ?? '';
            account.BillingCountry = account.ShippingCountry ?? '';

        }
    }
    ```

}
Comment on lines +1 to +42

Check warning

Code scanning / PMD

As triggers do not allow methods like regular classes they are less flexible and suited to apply good encapsulation style. Therefore delegate the triggers work to a regular class (often called Trigger handler class). See more here: <https://developer.salesforce.com/page/Trigger_Frameworks_and_Apex_Trigger_Best_Practices> Warning

Avoid logic in triggers
5 changes: 5 additions & 0 deletions force-app/main/default/triggers/AccountTrigger.trigger-meta.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<ApexTrigger xmlns="http://soap.sforce.com/2006/04/metadata">
<apiVersion>60.0</apiVersion>
<status>Active</status>
</ApexTrigger>
49 changes: 49 additions & 0 deletions force-app/main/default/triggers/OpportunityTrigger.trigger
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
trigger OpportunityTrigger on Opportunity (before delete, before update) {
if (Trigger.isBefore && Trigger.isDelete) {
// Question 6 solution
Set<Id> accIds = new Set<Id>();
for (Opportunity opp : Trigger.old) {
if (opp.AccountId != null) {
accIds.add(opp.AccountId);
}
Copy link

@bdJohnson72 bdJohnson72 May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is good. A different approach would be to add the ids where the Opp = closed won. So you only query for those.

}

Map<Id, Account> accMap = new Map<Id, Account>([SELECT Id, Industry FROM Account WHERE Id IN :accIds]);
Copy link

@bdJohnson72 bdJohnson72 May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I would add a where clause and filter for Industry = Banking


for (Opportunity opp : Trigger.old) {
if (opp.StageName == 'Closed Won') {
Account acc = accMap.get(opp.AccountId);
if (acc != null && acc.Industry == 'Banking') {
opp.addError('Cannot delete closed opportunity for a banking account that is won');
}
}
}
}

if (Trigger.isBefore && Trigger.isUpdate) {
// Question 5 solution
for (Opportunity opp : Trigger.new) {
if (opp.Amount < 5000) {
opp.addError('Opportunity amount must be greater than 5000');
}
Copy link

@bdJohnson72 bdJohnson72 May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good. I would check less than or equal too. My assumption since the error message says that the amount must be greater than

}

// Question 7 solution
Set<Id> accIds = new Set<Id>();
for (Opportunity opp : Trigger.new) {
accIds.add(opp.AccountId);
}

Map<Id, Contact> conIdMap = new Map<Id, Contact>();
for (Contact con : [SELECT Id, AccountId FROM Contact WHERE AccountId IN :accIds AND Title = 'CEO']) {
conIdMap.put(con.AccountId, con);
}

for (Opportunity opp : Trigger.new) {
Contact ceoCon = conIdMap.get(opp.AccountId);
if (ceoCon != null) {
Copy link

@bdJohnson72 bdJohnson72 May 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this value could ever be null.

opp.Primary_Contact__c = ceoCon.Id;
}
}
}
}
Comment on lines +1 to +60

Check warning

Code scanning / PMD

As triggers do not allow methods like regular classes they are less flexible and suited to apply good encapsulation style. Therefore delegate the triggers work to a regular class (often called Trigger handler class). See more here: <https://developer.salesforce.com/page/Trigger_Frameworks_and_Apex_Trigger_Best_Practices> Warning

Avoid logic in triggers
Comment on lines +1 to +60

Check warning

Code scanning / PMD

Complexity directly affects maintenance costs is determined by the number of decision points in a method plus one for the method entry. The decision points include 'if', 'while', 'for', and 'case labels' calls. Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote high complexity, and 11+ is very high complexity. Warning

The method 'invoke' has a Standard Cyclomatic Complexity of 12.
Comment on lines +1 to +60

Check warning

Code scanning / PMD

Complexity directly affects maintenance costs is determined by the number of decision points in a method plus one for the method entry. The decision points include 'if', 'while', 'for', and 'case labels' calls. Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote high complexity, and 11+ is very high complexity. Warning

The trigger 'OpportunityTrigger' has a Standard Cyclomatic Complexity of 13 (Highest = 12).
Comment on lines +1 to +60

Check warning

Code scanning / PMD

Methods that are highly complex are difficult to read and more costly to maintain. If you include too much decisional logic within a single method, you make its behavior hard to understand and more difficult to modify. Cognitive complexity is a measure of how difficult it is for humans to read and understand a method. Code that contains a break in the control flow is more complex, whereas the use of language shorthands doesn't increase the level of complexity. Nested control flows can make a method more difficult to understand, with each additional nesting of the control flow leading to an increase in cognitive complexity. Information about Cognitive complexity can be found in the original paper here: <https://www.sonarsource.com/docs/CognitiveComplexity.pdf> By default, this rule reports methods with a complexity of 15 or more. Reported methods should be broken down into less complex components. Warning

The trigger 'OpportunityTrigger' has a cognitive complexity of 30, current threshold is 15
Comment on lines +1 to +60

Check warning

Code scanning / PMD

The complexity of methods directly affects maintenance costs and readability. Concentrating too much decisional logic in a single method makes its behaviour hard to read and change. Cyclomatic complexity assesses the complexity of a method by counting the number of decision points in a method, plus one for the method entry. Decision points are places where the control flow jumps to another place in the program. As such, they include all control flow statements, such as 'if', 'while', 'for', and 'case'. Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote high complexity, and 11+ is very high complexity. By default, this rule reports methods with a complexity >= 10. Additionally, classes with many methods of moderate complexity get reported as well once the total of their methods' complexities reaches 40, even if none of the methods was directly reported. Reported methods should be broken down into several smaller methods. Reported classes should probably be Warning

The trigger 'OpportunityTrigger' has a cyclomatic complexity of 13.
5 changes: 5 additions & 0 deletions force-app/main/default/triggers/OpportunityTrigger.trigger-meta.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<ApexTrigger xmlns="http://soap.sforce.com/2006/04/metadata">
<apiVersion>60.0</apiVersion>
<status>Active</status>
</ApexTrigger>
Loading