✅ Will I fall into a gotcha with this constructor?

Hi, all: I'm trying to setup a class for managing my AWS S3 interactions, but I'm wondering if there are any issues with my definition:
C#
public class S3Service(ILogger<S3Service> logger, IAmazonS3 s3Client) : IS3Service
{
private readonly ILogger<S3Service> _logger = logger;
private readonly IAmazonS3 _s3Client = s3Client;
private readonly TransferUtility _transferUtility = new(s3Client);
...
C#
public class S3Service(ILogger<S3Service> logger, IAmazonS3 s3Client) : IS3Service
{
private readonly ILogger<S3Service> _logger = logger;
private readonly IAmazonS3 _s3Client = s3Client;
private readonly TransferUtility _transferUtility = new(s3Client);
...
Looks simple enough, but I'm worried about two things, TransferUtility inherits from IDisposable, will the framework properly handle the disposability of the _transferUtility? Also, would it be more appropriate to create a new TransferUtility object under the scope of each function that I want to use the class rather than creating a class field? For example:
C#
public Task MyUploadFunctionAsync()
{
using var transfer = new TransferUtility();
... transfer files
}
C#
public Task MyUploadFunctionAsync()
{
using var transfer = new TransferUtility();
... transfer files
}
I'll be expecting to upload many (~50 25MB) files at once. If the latter approach is more appropriate I can of course pass in a list of objects I'd like to upload. Thanks 🙂
20 Replies
SleepWellPupper
SleepWellPupper3mo ago
Since S3Service is newing up the TransferUtility, it owns its lifetime. Therefore, you should either implement IDisposable on S3Service, or move ownership by injecting TransferUtility instead of IAmazonS3.
MODiX
MODiX3mo ago
Sorry @C@NYON⭐ONL!NE your message contained blocked content and has been removed!
C@NYON⭐ONL!NE
C@NYON⭐ONL!NEOP3mo ago
Hmmm let me try messaging in parts
MODiX
MODiX3mo ago
Sorry @C@NYON⭐ONL!NE your message contained blocked content and has been removed!
C@NYON⭐ONL!NE
C@NYON⭐ONL!NEOP3mo ago
IDK why my message is blocked 😭
MODiX
MODiX3mo ago
Sorry @C@NYON⭐ONL!NE your message contained blocked content and has been removed!
C@NYON⭐ONL!NE
C@NYON⭐ONL!NEOP3mo ago
Whatever
Sehra
Sehra3mo ago
$paste
MODiX
MODiX3mo ago
If your code is too long, you can post to https://paste.mod.gg/, save, and copy the link into chat for others to see your shared code! Sorry @C@NYON⭐ONL!NE your message contained blocked content and has been removed!
SleepWellPupper
SleepWellPupper3mo ago
What're you trying to paste?
C@NYON⭐ONL!NE
C@NYON⭐ONL!NEOP3mo ago
🤷
No description
SleepWellPupper
SleepWellPupper3mo ago
What I mean by "injecting TransferUtility instead of IAmazonS3" is that you replace your constructor parameter from IAmazonS3 to TransferUtility and leave the disposal to whatever is creating your S3Service
Sehra
Sehra3mo ago
you can add it as services.AddScoped(sp => new TransferUtility(sp.GetRequiredService<IAmazonS3>()))
SleepWellPupper
SleepWellPupper3mo ago
if you resolve your service from a di container then that would be responsible for disposal. Other than that, your Dispose implementation is fine.
C@NYON⭐ONL!NE
C@NYON⭐ONL!NEOP3mo ago
okay with sehra's line of code i see what you mean. i use functions available in IAmazonS3 but not TransferUtility in some of my implementation, particularly for listing buckets and keys in them, so i'll have to go with the Dispose route i think
SleepWellPupper
SleepWellPupper3mo ago
yep, that makes sense. Still, you could inject both IAmazonS3 and TransferUtility and not deal with disposal in your service.
C@NYON⭐ONL!NE
C@NYON⭐ONL!NEOP3mo ago
that is tempting particularly because VS is warning me about CA1816: Call GC.SuppressFinalize correctly which is approaching ends of the language i don't fully comprehend
SleepWellPupper
SleepWellPupper3mo ago
forget about that public void Dispose() => foo.Dispose(); is fine if your disposable dependency (_transferUtility) is managed (i.e. implements IDisposable itself). It does, so you don't have to worry about the finalization stuff.
Unknown User
Unknown User3mo ago
Message Not Public
Sign In & Join Server To View
MODiX
MODiX3mo ago
If you have no further questions, please use /close to mark the forum thread as answered

Did you find this page helpful?