Testura/Testura.Code

Make the ICodeSaver.SaveCodeToFile method asynchronous

Opened this issue · 7 comments

SeanFarrow commented

it would be really nice if the ICodeSave.SaveCodeToFile method was made asynchronous. Or maybe we should add a new method and obsolete the old one?

jeffward01 commented

I think this is a great idea. Here is the PR for it: #101

It will go out in the next release once it is merged 😄

Great suggestion @SeanFarrow !! Please keep the suggestions coming!

Side note: I have a bunch of code and fluent syntax builders that I want to push, I will soon! I have not had time yet.

Thanks again for the suggestion here!

jeffward01 commented

I did notice this on it:

Let's say I have a directory that is like this /myDirectory/ with no files or sub-directories in it.

Then I pass in this path to CodeSaver: /myDirectory/someSubDirectory/myfile.cs

It will throw an error that it cannot find the path. This is Microsoft System.IO that throws the error, not us. Its just the nature of System.IO

What do you guys think about adding a check in there to ensure that it can write to the file? Or we can be more brutal and do something like this:

var fi = new FileInfo(path);
Directory.Create(fi.Directory.FullName);
// ~~~ continue to write the file

Thats a bit more invasive, but also will make peoples lives 'easier'. Maybe its better to do a check and throw an exception instead? What do you guys think @SeanFarrow @MilleBo

SeanFarrow commented
jeffward01 commented

@SeanFarrow - Done, I added the check on both SaveCodeToFileAsync and SaveCodeToFile

If you have any other ideas please feel free to share!

SeanFarrow commented

@jeffward01 The only things I might do differently are:
use the FormatAsync method of formatter, passing in a CalcellationToken if possible.
See whether we can replace the use of FileStreams/StreamWriter with File.WriteTextAsync.

Other than that, this looks really good.

  • 🎉1
jeffward01 commented

@SeanFarrow - no problem, I just implemented the SaveCodeToFileAsync as async because that was the one you had asked for.

I agree completely that we should implement async methods wherever possible. My personal preference is to implement both Sync and Async versions, because in my experience sometimes calling .GetAwaiter().GetResult(); is not the same as a Sync method.

Here is a good write-up about it:

The article above refrences this blog post, which is also a good write up:

I plan to sweep and implement async wherever possible, and always include the cancellationToken.

I saw in some microsoft documentation, that for public facing SDK's they were handling the cancellationToken like this:

public async Task SomeMethodAsync(string someParam, CancellationToken cancellationToken =  default)
{
  // This is the interesting part imo
  cancellationToken.ThrowIfCancellationRequested()

  // do work...

}

Thanks for the suggestion! when I have some time i'll implement more async methods. if there are any blockers for you, feel free to do the same and we'll get it pulled in!

SeanFarrow commented
  • 👍1