Skip to content

Commit

Permalink
C# [EC86] GC.Collect should not be called & [EC87] Use collection ind…
Browse files Browse the repository at this point in the history
…exer (#295)
  • Loading branch information
Djoums authored May 14, 2024
1 parent d2d19db commit 63303ed
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 8 deletions.
12 changes: 7 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- [#298](https://github.com/green-code-initiative/ecoCode/pull/298) Add HTML rule EC36 (Avoid autoplay for videos and audio content)
- [C# #36](https://github.com/green-code-initiative/ecoCode-csharp/issues/36) [EC86] [C#] GC.Collect should not be called
- [C# #42](https://github.com/green-code-initiative/ecoCode-csharp/issues/42) [EC87] [C#] Use collection indexer

### Changed

Expand All @@ -32,11 +34,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Swift rules cleanup and updates (removed duplicated rules, added [EC602])
- [#18](https://github.com/green-code-initiative/ecoCode-csharp/issues/18) [EC81] [C#] Specify struct layout
- [#285](https://github.com/green-code-initiative/ecoCode/pull/285) [EC82] [C#] Cariable can be made constant
- [#286](https://github.com/green-code-initiative/ecoCode/issues/286) [EC83] [C#] Replace Enum ToString() with nameof
- [#27](https://github.com/green-code-initiative/ecoCode-csharp/issues/27) [EC84] [C#] Avoid async void methods
- [#34](https://github.com/green-code-initiative/ecoCode-csharp/issues/34) [EC85] [C#] Make type sealed
- [C# #18](https://github.com/green-code-initiative/ecoCode-csharp/issues/18) [EC81] [C#] Specify struct layout
- [C# #285](https://github.com/green-code-initiative/ecoCode/pull/285) [EC82] [C#] Variable can be made constant
- [C# #286](https://github.com/green-code-initiative/ecoCode/issues/286) [EC83] [C#] Replace Enum ToString() with nameof
- [C# #27](https://github.com/green-code-initiative/ecoCode-csharp/issues/27) [EC84] [C#] Avoid async void methods
- [C# #34](https://github.com/green-code-initiative/ecoCode-csharp/issues/34) [EC85] [C#] Make type sealed

## [1.5.0] - 2024-02-02

Expand Down
2 changes: 2 additions & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ Some are applicable for different technologies.
| EC83 | Replace Enum ToString() with nameof | When no string format is applied, use nameof instead of ToString() for performance | | 🚫 | 🚫 | 🚫 | 🚫 | 🚫 || 🚫 |
| EC84 | Avoid async void methods | Use async Task methods instead, for performance, stability and testability | | 🚫 | 🚫 | 🚫 | 🚫 | 🚫 || 🚫 |
| EC85 | Make type sealed | Seal types that don't need inheritance for performance reasons | | 🚫 | 🚫 | 🚫 | 🚫 | 🚫 || 🚫 |
| EC86 | GC.Collect should not be called | In most cases, the cost of calling GC.Collect far outweighs the benefits | | 🚫 | 🚫 | 🚫 | 🚫 | 🚫 || 🚫 |
| EC87 | Use collection indexer | Collection indexers should be used instead of Linq, when available | | 🚫 | 🚫 | 🚫 | 🚫 | 🚫 || 🚫 |
| EC203 | Detect unoptimized file formats | When it is possible, to use svg format image over other image format | | 🚀 | 🚀 | 🚀 || 🚀 | 🚀 | 🚫 |
| EC404 | Avoid list comprehension in iterations | Use generator comprehension instead of list comprehension in for loop declaration | | 🚫 | 🚫 | 🚫 || 🚫 | 🚫 | 🚫 |
| | Use official social media sharing buttons | These JavaScript plugins are very resource-intensive: to work, they require a large number of requests and download heavy files. It is better to prefer direct links. | [cnumr best practices (3rd edition) BP_019](https://github.com/cnumr/best-practices/blob/main/chapters/BP_019_fr.md) | 🚫 | 🚫 | 🚀 | 🚫 | 🚫 | 🚫 | 🚀 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ public class Class1 // Non-compliant if not inherited from, make type sealed
public class Class2 // Non-compliant if not inherited from, make type sealed
{
public sealed void Method(); // The sealed keyword will be removed by the code fix
public sealed void Method();
}
internal class Class3 // Non-compliant if not inherited from because the type is not public, even with a virtual method
{
public virtual void Method(); // The virtual keyword will be removed by the code fix
public virtual void Method();
}
public class Class4 // Compliant even if not inherited from, the virtual method hints at being overridable from other assemblies
Expand All @@ -46,7 +46,7 @@ public class Class4 // Compliant even if not inherited from, the virtual method
public class Class5 : Class4 // Non-compliant if not inherited from, make type sealed
{
public sealed override void Method(); // The sealed keyword will be removed by the code fix
public sealed override void Method();
}
public class Class6 : Class4 // Compliant, Method() is still overridable
Expand Down
15 changes: 15 additions & 0 deletions ecocode-rules-specifications/src/main/rules/EC86/EC86.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"title": "GC.Collect should not be called",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "10min"
},
"tags": [
"eco-design",
"ecocode",
"bad-practice"
],
"defaultSeverity": "Major"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
:!sectids:

`GC.Collect` should not be called.

## Why is this an issue ?

Calling `GC.Collect` forces a garbage collection to occur. This can be a very expensive operation, as it is fully blocking and can take a significant amount of time to complete.
In most cases, the garbage collector is able to determine when it is appropriate to run a collection, and calling `GC.Collect` is not only unnecessary but also not advisable.

Calling `GC.Collect` on generation 0 (ephemeral objects) is excluded from this rule, as it is very lightweight in comparison to the other generations.

### When can it be ignored ?

In general, it is not recommended to call GC.Collect as the cost far outweighs the benefits.
In some cases however, typically if you know you have a lot of long-lived objects whose memory you need reclaimed immediately, calling `GC.Collect` can make sense.

## Non-compliant examples

[source, cs]
----
public void Method();
{
GC.Collect(); // Non-compliant, same as GC.Collect(generation: GC.MaxGeneration)
GC.Collect(generation: 2); // Non-compliant
}
----

## Compliant examples

[source, cs]
----
public void Method();
{
GC.Collect(generation: 0); // Compliant
GC.Collect(generation: 0, mode: GCCollectionMode.Optimized); // Compliant
}
----
16 changes: 16 additions & 0 deletions ecocode-rules-specifications/src/main/rules/EC87/EC87.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"title": "Use collection indexer",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"eco-design",
"ecocode",
"performance",
"bad-practice"
],
"defaultSeverity": "Major"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
:!sectids:

Use collection indexer.

## Why is this an issue ?

Linq methods like `First()`, `Last()` and `ElementAt()` can be necessary on enumerable types that don't have an indexer.
But for those that implement `IReadOnlyList<T>`, `IList<T>` or `IList`, direct index access is cheaper at runtime and should be used instead.

### When can it be ignored ?

This rule shouldn't be ignored.

## Non-compliant examples

[source, cs]
----
public static void Test(int[] arr)
{
int first = arr.First(); // Non-compliant, use arr[0]
int last = arr.Last(); // Non-compliant, use arr[^1], or arr[arr.Length - 1] if C# < 8
int third = arr.ElementAt(2); // Non-compliant, use arr[2]
}
----

## Compliant examples

[source, cs]
----
public static void Test(List<int> list)
{
int first = list[0];
int last = list[^1]; // Or list[list.Count - 1] if C# < 8
int third = list[2];
}
----

0 comments on commit 63303ed

Please sign in to comment.