CODEWITHSUNDEEP
c#oopmethodsaccessor
Sj03rs
Sj03rs

Reputation: 937

Calling a method inside an accessor C#

So I have been struggling with this for quite a while.
I am trying to learn OOP but this is so big that it is hard for me at least to understand all of it.
Let's say I have a method in a class that gets something from a database.
This data will be set to a variable.
What I do now is have a public method, call it, and then call the accessor with the value. For example:

        private string _name;

        public string Name { get { return _name; } }

        public void MyMethod()
        {
            using (SqlConnection connect = new SqlConnection(connectionString))
            {
                using (SqlCommand command = new SqlCommand("SELECT Name FROM Table WHERE ID = 1", connect))
                {
                    connect.Open();
                    using (SqlDataReader reader = command.ExecuteReader())
                    {
                        if (reader.Read())
                        {
                            _name = Convert.ToString(reader["Name"]);
                        }
                    }
                }
            }
        }

And I would call it like:

MyClass myclass = new MyClass();
myclass.MyMethod();
string myname = myclass.Name;

Now this seems really weird to me.
So I thought, maybe it would be worth it to call the method inside the accessor.
Like:

public string Name { get { MyMethod(); return _name; } }

Calling it like:

MyClass myclass = new MyClass();
string myname = myclass.Name;

This would result in less code and I can make my method private.
Yet, I would have trouble doing this when the method has parameters to it.
So how do I do this? Is this bad practice? If this is not how I should do this, then how do I do it?

I would really love to hear from you guys! Please don't hesitate to ask questions or make changes to my question.
Also, my sincere apologies if this has been asked already.
Any great tutorials are also really welcome!

Upvotes: 2

Views: 10439

Answers (3)

MrPaulch
MrPaulch

Reputation: 1418

What you are implementing is a form of Lazy Evaluation that is sometimes called cached property

The general design pattern is:

private SomeType _value;
public SomeType CachedProperty {
     get {
          if(_value == null) {
              _value = GetValue(); // slow
          }
          return _value;
     }
}

There is a few reasons that discourage the notion of this kind of property:

  • GetValue() is typically slow. Thats why we cashe it's value in the first place. So accessing the property is only slow on the first call. But we expect a property to be always fast.

  • The cashed value might be obsolete by the time we need it again. This can be mitigated by a public method like UpdateValue. But we expect a property to be always up to date.

  • If you omit the _value == null check, then the property becomes a wrapper for a method. Why are we using a property then at all?

So essentially this design patter hides the fact that the underlying value is volatile in nature and might cost some time to retreive.

Whether you really want to do this, depends on the demands of the code surrounding you.

But in general I would just go with a typical async pattern:

public SomeType GetValue() { ... }
public Task<SomeType> GetValueAsync() { ... }

and if you really need to work with a cached value on different scopes add a:

public SomeType CachedValue { get; }

This clarifies that the value that you get the "fast way", might be outdated.

Upvotes: 4

YoryeNathan
YoryeNathan

Reputation: 14512

  • You could consider running the method in the ctor, once.

  • You could have a public RefreshData method which will update all you need from the db.

  • You could have the property check whether the _name is null, and only if it is - it would update it from database and then return it - otherwise it just returns the existing value. You can combine this with a Invalidate method, which will set _name and any other relevant member to null or some other indication, to cause their properties to indeed re-call the update function when they are next called.

In real life cases, you will usually know what's the better approach - but they are all OOP.

However the solution you suggested is somewhat against the OOP principles, since the MyMethod should probably not be exposed, and your class is probably the one that is responsible for it's data.

Also as Jamiec's answer suggests, you should implement the DB-related logic elsewhere, in a Repository class or even a whole layer (separate project sometimes) just for that (look for DAL). This is because that even though your class might be responsible for it's own data - it should not "know" anything about databases or query languages. At most - it knows how to ask something independent of it for some data.

Upvotes: 3

Jamiec
Jamiec

Reputation: 136094

The important thing to note is that these 2 things (A object holding your data and the method of getting that data) are 2 very different concerns. Good practice dictates that your classes have a separation of concerns.

In your case, you should have a "repository" class which knows how to get data from a database and a DTO (or, simply, an Object) which has the properties. Your repository will likely load all the data required and return an object with all the data present.

Upvotes: 1

Related Questions