stride icon indicating copy to clipboard operation
stride copied to clipboard

Easy One - Avoid Exception when AppSettings File is missing

Open najak3d opened this issue 2 years ago • 3 comments

Here's a quick code change that I'd like to submit to Stride codebase -- just a few lines of code, to eliminate a pointless FileNotFound Exception at startup, from calling "YamlSerializer.Load<T>".

Here's the code "as is now" from AppSettingsProvider:

image

Here's the code after I edited it:

image

What I did matches how it's done from the only other place in code that we call this method, where the File.Exists is checked beforehand to avoid using Exception for Program Flow:

image

So this just requires a few lines of code to change, and poof, it behaves the SAME but avoid throwing an exception.

najak3d avatar Apr 06 '22 07:04 najak3d

It behaves the same for the vast majority of cases but it's not sufficient as the file could be moved between the Exists test and the read inside of Load. Yes it is very unlikely to ever happen but this is also not a bug, so if changes have to be made they must either improve the program or at the very least can't introduce a potential bug, you'll have to find another way.

Eideren avatar Apr 06 '22 14:04 Eideren

OK, then I'd like to modify my request to simply insert the "File.Exists()" call to prevent exceptions from being "normal program flow". Then the exception would only happen that 1 in a billion times where the File was deleted inside this 100 nanosecond window....

So this provides 100% same safety, while NOT using Exceptions for normal program flow.

image

najak3d avatar Apr 06 '22 19:04 najak3d

LGTM and that exception is a nuisance. I'd say go ahead and make a PR for the change.

jrinker03 avatar Apr 18 '22 22:04 jrinker03