Git Product home page Git Product logo

Comments (10)

perkops avatar perkops commented on July 30, 2024 3

Are we still sure on this?

E.g. in a console app:

services.AddLogging(logging =>
{
     logging.AddConsole().SetMinimumLevel(LogLevel.Information);
     logging.AddDebug().SetMinimumLevel(LogLevel.Trace);
});

Becomes

_ = services.AddLogging(logging =>
{
     _ = logging.AddConsole().SetMinimumLevel(LogLevel.Information);
     _ = logging.AddDebug().SetMinimumLevel(LogLevel.Trace);
});

from atc-coding-rules.

JKrag avatar JKrag commented on July 30, 2024 2

Another example that will get a bit uglier with this rule:

    requestHeaders.Add("Accept");
    requestHeaders.Add("Accept-Encoding");
    requestHeaders.Add("Cache-Control");
    requestHeaders.Add("Pragma");
    requestHeaders.Add("Upgrade-Insecure-Requests");
    requestHeaders.Add("User-Agent");
    requestHeaders.Add("X-Forwarded-Host");
    requestHeaders.Add("X-Forwarded-Proto");

They all just return a true/false depending on whether the element was already in the hash set.

Does changing all of these to:

 _ = requestHeaders.Add("X-Forwarded-Proto");

really provide value that counters the additional verbosity? maybe it does, I am not sure. I just wanted to add the examples to the mix for consideration.

The same goes for e.g. services:

    services.AddSingleton<Utilities>();
    services.AddScoped<IUpdater, UpdaterService>();
    services.AddScoped<IFileUtility, FileUtility>();
    services.AddScoped<IUriHelper, UriHelper>();

from atc-coding-rules.

egil avatar egil commented on July 30, 2024 1

Decision: ROOT = NONE.

Refer to arguments above, where e.g. build pattern based code suffer, we have reversed to NONE.

from atc-coding-rules.

cjakobsen avatar cjakobsen commented on July 30, 2024

I find this to be a good rule, you can see that the author has thought about the return value and not just forgotten about it. I vote for keeping it.

from atc-coding-rules.

egil avatar egil commented on July 30, 2024

I find this to be a good rule, you can see that the author has thought about the return value and not just forgotten about it. I vote for keeping it.

Do you also see the value of that in tests? I don't want to disable this in production code, just test code.

from atc-coding-rules.

egil avatar egil commented on July 30, 2024

To show an example, where think this rule doesnt add value, consider this:

[Theory, AutoNSubstituteData]
public async Task MasterAssigned_Doesnt_Change_Master_When_Recipe_Does_Not_Exists(
    [Frozen] IRecipeRepository repository,
    RecipeBuilder sut,
    MasterAssigned @event,
    CancellationToken cancellationToken)
{
    repository.GetByIdAsync(default, default).ReturnsForAnyArgs(default(Recipe));

    await sut.ExecuteEventAsync(@event, cancellationToken);

    _ = repository.DidNotReceive().StoreAsync(Arg.Any<Recipe>(), Arg.Is(cancellationToken));
    _ = repository.DidNotReceive().StoreAsync(Arg.Any<Recipe[]>(), Arg.Is(cancellationToken));
}

vs this:

[Theory, AutoNSubstituteData]
public async Task MasterAssigned_Doesnt_Change_Master_When_Recipe_Does_Not_Exists(
    [Frozen] IRecipeRepository repository,
    RecipeBuilder sut,
    MasterAssigned @event,
    CancellationToken cancellationToken)
{
    repository.GetByIdAsync(default, default).ReturnsForAnyArgs(default(Recipe));

    await sut.ExecuteEventAsync(@event, cancellationToken);

    repository.DidNotReceive().StoreAsync(Arg.Any<Recipe>(), Arg.Is(cancellationToken));
    repository.DidNotReceive().StoreAsync(Arg.Any<Recipe[]>(), Arg.Is(cancellationToken));
}

To me, this is very much the same consideration as with string.Equals vs ==. Sure, with _ = I make it explicit that I do not care about the value. But to be honest, that should be obvious in this context.

from atc-coding-rules.

egil avatar egil commented on July 30, 2024

Decision: ROOT = ERROR

from atc-coding-rules.

davidkallesen avatar davidkallesen commented on July 30, 2024

https://github.com/atc-net/atc-coding-rules/blob/main/documentation/CodeAnalyzersRules/MicrosoftCodeAnalysis/IDE0058.md

from atc-coding-rules.

egil avatar egil commented on July 30, 2024

This rules is starting to annoy more and more in production code as well, e.g. here is an example of a fluent interface/api where it complains when we do not continue the fluent chain...:

image

Has to be this to make things compile:

image

from atc-coding-rules.

davidkallesen avatar davidkallesen commented on July 30, 2024

Commit: 522c1b9

from atc-coding-rules.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.