Random opinionated changes #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A clean error list is a happy error list
There is no need to check if a string is a simple type, only to then check if its a string. Additionally, by capturing the result of the type check, we avoid the extra as (or cast)
At the end of the day, what we want here is null warning suppression, but doing it with an exclamation mark can be quite hidden, and can just result in exceptions sometime later at runtime. The AssumeNotNull method is an aggressive version that throws at the site of the actual null. Also increased the language version to be able to use nameof to refer to a parameter.
The system never puts nulls in, so it was a little odd that it allowed for them to be retreived. Instead, if a message can't be retreived then the whole message may as well be null.
C# 12 is nce
The BeginScope method is on ILogger, not ILogger<T>, so there was no difference between this and the method below
In particular with Nullable<T> its much nicer to use patterns, rather than .HasValue and then .Value. There is also a minor perf improvement as .Value has to check, and throw, for nulls.
Task.WhenAll returns an array, but returning it as an IEnumerable meant the GetMessageAsync method has to enumerate the results no less than 4 times!
Mainly I wanted to remove yet another enumeration of a collection, but also because this calculation seemed useless to me. If we want to batch into chunks of 20, and there are only 10, then surely it will produce one chunk whether the batch size is set to 10 or 20.
The IDE will automatically show the doc comments from the interface member, without needing to specify inheritdoc everywhere. Also removed some whitespace I saw while checking that no, this isn't producing a documentation file.
Sealed performs better 😛
This is the convention, and clearly communicates intent
Default interface implementations are tempting to use as helper methods, but they are not great, as any new implementor will, at best, think they have to implement them, and at worst get compile errors. Helpers written as extension methods in the same namespace are just as discoverable, and should work regardless of implementation.
Must have crept in when rebasing
No need to force things to stay on the same thread when doing async network I/O to Azure
Why not go all out :D
davidwengier
force-pushed
the
OpinionsAreLike
branch
from
0ca6cf5 to
202bf86
Compare
It didn't build in a codespace, so I guess this is too new.
Codecov ReportAttention:
📢 Thoughts on this report? Let us know! |
PureKrome
reviewed
Nov 9, 2023
src/SimpleAzure.Storage.HybridQueue/SimpleAzure.Storage.HybridQueue.csproj
Show resolved
Hide resolved
…re places to update if anything changes.
Check out how awesome patterns are! Didn't have to update any of them even though the return type now has a Count property instaed of a Length property! Yay patterns!!
PureKrome
approved these changes
Nov 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No idea if this works, because I couldn't run tests locallyTests pass in a codespaceReviewing commit-at-a-time will explain what each change is. Feel free to not take any of this if you don't like it :)