-
Notifications
You must be signed in to change notification settings - Fork 12
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
Implement LinkedMap class #124
base: main
Are you sure you want to change the base?
Conversation
Rollback Files For Sweep
|
Apply Sweep Rules to your PR?
|
WalkthroughThis changeset introduces a new class Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files selected (2)
- force-app/main/default/classes/LinkedMap.cls (1)
- force-app/main/default/classes/LinkedMapTests.cls (1)
Files skipped from review due to trivial changes (1)
- force-app/main/default/classes/LinkedMapTests.cls
Review comments generated (6)
- Review: 3
- LGTM: 3
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
public List<Object> get(Set<String> keys) { | ||
return this.linkedMap.get(keys); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The get
method returns null when the keys are not found in the map. It would be better to return an empty list instead to avoid potential NullPointerExceptions.
public List<Object> get(Set<String> keys) {
- return this.linkedMap.get(keys);
+ return this.linkedMap.getOrDefault(keys, new List<Object>());
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a thing.
Want to put a null-safe operator @codefriar ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Wrote Changes
Done.
public List<Object> remove(Set<String> keys) { | ||
return this.linkedMap.remove(keys); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the get
method, the remove
method also returns null when the keys are not found in the map. Consider returning an empty list instead.
public List<Object> remove(Set<String> keys) {
- return this.linkedMap.remove(keys);
+ return this.linkedMap.remove(keys) != null ? this.linkedMap.remove(keys) : new List<Object>();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files selected (1)
- force-app/main/default/classes/LinkedMap.cls (1)
Review comments generated (8)
- Review: 2
- LGTM: 6
Tips
Chat with CodeRabbit Bot (@coderabbitai
)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitai
in a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignore
anywhere in the PR description to pause further reviews from the bot.
public void put(Set<String> keys, List<Object> values) { | ||
if (this.linkedMap.containsKey(keys)) { | ||
this.linkedMap.get(keys).addAll(values); | ||
} else { | ||
this.linkedMap.put(keys, values); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The put
method currently allows for duplicate values to be added to the list of values for a given key set. If this is not the intended behavior and you want to prevent duplicates, consider using a Set<Object>
instead of a List<Object>
. However, if maintaining the order of insertion is important, then the current implementation is fine.
public List<List<Object>> values() { | ||
return this.linkedMap.values(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the values()
method, you are returning the original list of values from the map. This could potentially allow outside code to modify the internal state of your LinkedMap
object. To prevent this, consider returning a deep copy of the values.
- public List<List<Object>> values() {
- return this.linkedMap.values();
- }
+ public List<List<Object>> values() {
+ List<List<Object>> deepCopyValues = new List<List<Object>>();
+ for (List<Object> valueList : this.linkedMap.values()) {
+ deepCopyValues.add(new List<Object>(valueList));
+ }
+ return deepCopyValues;
+ }
public class LinkedMap { | ||
private Map<Set<String>, List<Object>> linkedMap; | ||
|
||
public LinkedMap() { | ||
this.linkedMap = new Map<Set<String>, List<Object>>(); | ||
} | ||
|
||
public void put(Set<String> keys, List<Object> values) { | ||
if (this.linkedMap.containsKey(keys)) { | ||
this.linkedMap.get(keys).addAll(values); | ||
} else { | ||
this.linkedMap.put(keys, values); | ||
} | ||
} | ||
|
||
public List<Object> get(Set<String> keys) { | ||
return this.linkedMap.containsKey(keys) ? this.linkedMap.get(keys) : new List<Object>(); | ||
} | ||
|
||
public List<Object> remove(Set<String> keys) { | ||
return this.linkedMap.remove(keys); | ||
} | ||
|
||
public Boolean containsKey(Set<String> keys) { | ||
return this.linkedMap.containsKey(keys); | ||
} | ||
|
||
public Set<Set<String>> keySet() { | ||
return this.linkedMap.keySet(); | ||
} | ||
|
||
public List<List<Object>> values() { | ||
return this.linkedMap.values(); | ||
} | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public LinkedMap() { | ||
this.linkedMap = new Map<Set<String>, List<Object>>(); | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public void put(Set<String> keys, List<Object> values) { | ||
if (this.linkedMap.containsKey(keys)) { | ||
this.linkedMap.get(keys).addAll(values); | ||
} else { | ||
this.linkedMap.put(keys, values); | ||
} | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public List<Object> get(Set<String> keys) { | ||
return this.linkedMap.containsKey(keys) ? this.linkedMap.get(keys) : new List<Object>(); | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public List<Object> remove(Set<String> keys) { | ||
return this.linkedMap.remove(keys); | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public Boolean containsKey(Set<String> keys) { | ||
return this.linkedMap.containsKey(keys); | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public Set<Set<String>> keySet() { | ||
return this.linkedMap.keySet(); | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
public List<List<Object>> values() { | ||
return this.linkedMap.values(); | ||
} |
Check warning
Code scanning / PMD
Missing ApexDoc comment Warning
PR Feedback (click)
Description
This PR implements a new class called
LinkedMap
that provides a map-like data structure with a set of keys and a list of values that are linked. The class adheres to the limitations of an Apex defined type.Summary of Changes
LinkedMap.cls
in theforce-app/main/default/classes/
directory.linkedMap
of typeMap<Set<String>, List<Object>>
to store the key-value pairs.put
,get
,remove
,containsKey
,keySet
, andvalues
in theLinkedMap.cls
class to provide functionality for adding, retrieving, removing, checking, and retrieving all keys and values in the map.LinkedMapTests.cls
in theforce-app/main/default/classes/
directory to cover all methods in theLinkedMap.cls
class with comprehensive test cases.Please review and merge this PR to incorporate the new
LinkedMap
class into the repository.Fixes #123.
🎉 Latest improvements to Sweep:
💡 To get Sweep to edit this pull request, you can:
Summary by CodeRabbit
LinkedMap
class that provides a map-like data structure with linked keys and values. This feature enhances data handling by allowing users to add, retrieve, remove, and check elements in the map more efficiently.LinkedMapTests
class to ensure the robustness of the newLinkedMap
class. The tests cover all key functionalities including adding, retrieving, removing, and checking elements in the map.