No Object Created To Be Dropped

ID

csharp.no_object_created_to_be_dropped

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

allocation, code_smell, dead-code, suspicious-statement

Description

Reports a new T(…​) that makes up an entire statement, so the object just built is assigned to nothing, returned nowhere and passed to nobody.

Rationale

An allocation whose result goes nowhere buys nothing. Either the statement is dead code that a reader has to puzzle over, or it is unfinished — and the most common unfinished form is a forgotten throw. new ArgumentException("id"); compiles without a warning, so the method keeps running with exactly the invalid input the author meant to reject, and the caller sees a confusing failure much later.

If a constructor’s side effects really are the reason for the statement, that intent is invisible at the call site. Move the work into a named method whose signature says what happens.

A creation dropped inside a test or lifecycle method ([Fact], [Test], [SetUp], …​) is not reported: xUnit has no Assert.DoesNotThrow, so the standard idiom for asserting that a call does not throw is to invoke it with no assertion — the test only fails if an exception propagates.

public class EntryTests
{
    [Fact]
    public void Ctor_ShouldNotThrow()
    {
        new Entry("id");   // OK — xUnit no-throw idiom, this is a test method
    }
}
public class Registrar
{
    public void Register(string id)
    {
        if (id == null)
        {
            new ArgumentNullException(nameof(id));   // FLAW — the exception is built, never thrown
        }

        new Entry(id);                               // FLAW — the entry is dropped on the spot
    }

    public Entry RegisterOk(string id)
    {
        if (id == null)
        {
            throw new ArgumentNullException(nameof(id)); // OK, the exception is thrown
        }

        Entry entry = new Entry(id);                 // OK, the instance is kept
        return entry;                                // OK, and handed to the caller
    }
}

Remediation

Decide what the object was for. If it is an exception, add the missing throw. If it is a value the code needs, assign it, return it or pass it on. If the statement is genuinely useless, delete it. If the constructor performs work that must happen, expose that work as a named method and call that instead.

References