-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
base: feedback
Are you sure you want to change the base?
Feedback #1
Changes from all commits
04b8e4b
2ee852d
c3ee2b6
1b29c53
e9cadfb
0d34c0e
d395a08
21ba6b9
6c6c3f7
fee9215
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 warningCode 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.
|
||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| } | ||
|
Comment on lines
+1
to
+42
Check warningCode 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
|
||
| 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> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| trigger OpportunityTrigger on Opportunity (before update, before delete) { | ||
|
|
||
| switch on Trigger.operationType { | ||
|
|
||
| when BEFORE_DELETE { | ||
|
|
||
| //Solution 6 | ||
| //Get account ids of opportunities to be deleted | ||
| Set<Id> accountIds = new Set<Id>(); | ||
| for (Opportunity opp : Trigger.old) { | ||
| if (opp.StageName == 'Closed Won') { | ||
| accountIds.add(opp.AccountId); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
|
||
| //Create a map of accounts based on the accountIds | ||
| Map<Id, Account> accountToIdMap = new Map<Id, Account>([SELECT Id, Industry FROM Account WHERE Id IN :accountIds]); | ||
|
|
||
| for (Opportunity opp : Trigger.old) { | ||
| if (opp.StageName == 'Closed Won') { | ||
| Account acc = accountToIdMap.get(opp.AccountId); | ||
| if (acc.Industry == 'Banking') { | ||
| opp.addError('Cannot delete closed opportunity for a banking account that is won'); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| when BEFORE_UPDATE { | ||
| //Solution 5 | ||
| for (Opportunity opp : Trigger.new) { | ||
| if (opp.Amount == null || opp.Amount <= 5000) { | ||
| opp.Amount.addError('Opportunity amount must be greater than 5000'); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
|
||
| //Solution 7 | ||
| 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) { | ||
| opp.Primary_Contact__c = ceoCon.Id; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| when else { | ||
| System.debug('OpportunityTrigger WHEN ELSE ACTIVATED.'); | ||
Check warningCode 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 warningCode 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.
|
||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+1
to
+60
Check warningCode 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 warningCode 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 warningCode 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 warningCode 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 warningCode 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.
|
||
| 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> |
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