An object reference is required for the non-static field, method, or property 'CustDAL.AddNewCust(ref Customer)

Question

For this code row

bool rowsAffected = CustDAL.AddNewCust(ref newCustomer);

I get an error:

An object reference is required for the non-static field, method, or property 'CustDAL.AddNewCust(ref Customer)

And the code in the CustDAL looks like

 public bool AddNewCust(ref Models.Customer newCustomer)
    {
        SqlCommand command = new SqlCommand
        {
            CommandText = string.Format("INSERT INTO CUSTOMER VALUES('{0}', '{1}', '{2}', '{3}','{4}')", newCustomer.Title, newCustomer.Forename, newCustomer.Surname, newCustomer.Address, newCustomer.PhoneNumber)
        };

        return ExecuteNonQuery(command);
    }

Zohar Peled · Answer 1 · 2018-04-02T16:07:49.930

This code is terrible on a few different levels.

First, it's an open door for SQL Injection attacks.
If anyone would set the customer's Title to, say, Title');DROP TABLE CUSTUMER;--, care to guess what happens next?
Never concatenate strings with user input to create SQL statement. Instead, use parameterized queries to send the user input to the database safely. A correct insert statement looks like this:

INSERT INTO TableName(Column1, Column2....Columnn) VALUES(@Param1, @Param2...@Paramn)

Second, if you add or remove a column from the Customer table, your code will break. Always specify the columns list when writing insert statements.
Imaging adding a new column to the Customer table for middle name. If you do that, your current statement will break, since the number of columns will stop matching the number of values.

Third, If any of the string properties of newCustomer contains the ' char you'll get an exception - since your SQL would become something like this:

INSERT INTO CUSTOMER VALUES('My title isn't that smart', 'My forename',...

Fourth, you are passing an instance of SqlCommand around, you don't dispose it. While it's not terrible not to dispose SqlCommand since it does not hold any unmanaged resources, that is an implementation detail. The fact is that it's implementing the IDisposable interface, and as such should be disposed. You can read a bit more about that on this SO post. best practice usage of instances that implements the IDisposable interface is as a local variable inside a using statment -

using(var cmd = new SqlCommand(sql, con))
{
    //... do command stuff here
}

Fifth, DbCommand.ExecuteNonQuery returns an int for a reason.
The return value of the ExecuteNonQuery is indicating the number of rows effected by the SQL Statement.
If you use a query that tries to insert multiple records, or update multiple records, you might need to know how may records where actually inserted or updated as a result.
Returning bool hides that data from the calling code.

Sixth, No point of passing the newCustomer parameter by reference, unless the method initializes a new instance of Models.Customer and assign it to the newCustomer reference.

So a better implementation of your AddNewCust method would probably look more like this:

public int AddNewCust(ref Models.Customer newCustomer)
{
    var sql = "INSERT INTO CUSTOMER (Title, Forename, Surname, Address, PhoneNumber) VALUES (@Title, @Forename, @Surname, @Address, @PhoneNumber)";
    using(var command = new SqlCommand(sql))
    {
        command.Parameters.Add("@Title", SqlDbType.VarChar).Value = newCustomer.Title;
        command.Parameters.Add("@Forename", SqlDbType.VarChar).Value = newCustomer.Forename;
        command.Parameters.Add("@Surname", SqlDbType.VarChar).Value = newCustomer.Surname;
        //  fill in the rest of the parameters here...
        return ExecuteNonQuery(command); // change this method to return int...
    };
}

All that being said, the cause of the current exception you get is because you try to execute an instance member as if it was a static member - I'm guessing CustDAL is the name of the class, and not the name of a reference to an instance of that class.
For more information, read this SO post You should first create an instance of the CustDAL class and only then you can access it's non-static members. Since DAL classes should probably be used throughout the application, you should probably have a field containing that reference in the calling class:

private CustDAL _custDal;

// in the constructor:

_custDal = new CustDAL();

// when you want to add a new customer:

var rowsAffected = _custDal.AddNewCust(newCustomer);

An object reference is required for the non-static field, method, or property 'CustDAL.AddNewCust(ref Customer)

1 Answers1