Best practice with get

[aine_canby]

Ms defines get as follows: an accessor method in a property or indexer
that retrieves the value of the property or the indexer element.

So is the following TestDirectory get bad practice?

class Framework
{
string directory;

static string testFolder = "Test";
public static string TestFolder { get { return testFolder; }}

public static string TestDirectory{ get { return directory
+@"\"+testFolder; }}

public Framework(string directory)
{
this.directory = directory;
}
}

should I do this instead:

public static string GetTestDirectory()
{
return directory+@"\"+testFolder;
}

Thanks,

Aine

 

[Jon Skeet [C# MVP]]

Ms defines get as follows: an accessor method in a property or indexer
that retrieves the value of the property or the indexer element.

So is the following TestDirectory get bad practice?

<snip>

Seems fine to me, although TestDirectory and TestFolder aren't very
clearly distinguished - I wouldn't know what to expect the difference
to be just based on the name.

Jon

 

[John Duval]

Ms defines get as follows: an accessor method in a property or indexer
that retrieves the value of the property or the indexer element.

So is the following TestDirectory get bad practice?

class Framework
{
string directory;

static string testFolder = "Test";
public static string TestFolder { get { return testFolder; }}

public static string TestDirectory{ get { return directory
+@"\"+testFolder; }}

public Framework(string directory)
{
this.directory = directory;
}

}

should I do this instead:

public static string GetTestDirectory()
{
return directory+@"\"+testFolder;

}

Thanks,

Aine

Hi Aine,
I think it's fine to make it a property. One minor change is that I
would probably use Path.Combine( ) to concatenate directory and
testFolder (instead of +@"\"+), which will handle cases where
directory has a trailing backslash. I would also probably save the
result in a member varialbe so you don't end up combining the path
every time someone calls Framework.TestDirectory.
John

 

[aine_canby]

<snip>

Seems fine to me, although TestDirectory and TestFolder aren't very
clearly distinguished - I wouldn't know what to expect the difference
to be just based on the name.

Jon

..
Yeah, I agree but the comments will clear that up. For me, a path is a
directory or file, a directory is: C:/folder1/folder2/folder3, and a
folder is folder1, folder2 or folder3.

 

[=?ISO-8859-1?Q?G=F6ran_Andersson?=]

One minor change is that I
would probably use Path.Combine( ) to concatenate directory and
testFolder (instead of +@"\"+), which will handle cases where
directory has a trailing backslash. I would also probably save the
result in a member varialbe so you don't end up combining the path
every time someone calls Framework.TestDirectory.

I was thinking of the exact same two things when reading the code.

 

[Jon Skeet [C# MVP]]

On Oct 24, 1:29 pm, (e-mail address removed) wrote:

Yeah, I agree but the comments will clear that up. For me, a path is a
directory or file, a directory is: C:/folder1/folder2/folder3, and a
folder is folder1, folder2 or folder3.

Just be aware that your terminology isn't universally known -
"relative" and "absolute" are more common ways of describing the
difference.

Jon

 

[Jon Skeet [C# MVP]]

I think it's fine to make it a property. One minor change is that I
would probably use Path.Combine( ) to concatenate directory and
testFolder (instead of +@"\"+), which will handle cases where
directory has a trailing backslash.
Agreed.

I would also probably save the
result in a member varialbe so you don't end up combining the path
every time someone calls Framework.TestDirectory.

That sounds like a premature optimisation to me - I'd wait until there
was any indication that it was a problem before doing that. In
particular, if you're finding a directory name you're probably going
to do some IO, and that's bound to dwarf the cost of Path.Combine.

Jon

 

[Larry Smith]

I would also probably save the result in a member

varialbe so you don't end up combining the path
every time someone calls Framework.TestDirectory.

This is one of the vexing problems about using properties IMHO. It's
tempting to apply get/set to anything that has the semantics of a property.
If the property does much more than just return a member variable however
then it can become expensive to use. Users of the property may not be aware
of this expense however nor should they. That is, because it's a property
and because it's syntactically so easy to use, it's just assumed to be
cheap. People will therefore (often) call it frequently and/or multiple
times within the span of several lines. The onus is therefore on the
programmer to:ensure that the property *is* cheap. This may not be
possible in all cases however (e.g., the value has to be dynamically
calculated or retrieved on each call), or you're otherwise forced to cache
the value in a member variable. In the former case you may be forced to
abandon the property in favour of a regular function and in the latter case
you may need to do the same thing (rather than cache every property since it
may create unaceptable overhead/clutter). In either case you may be forced
to use a regular function where a property really appears to be the more
natural choice. It's an inherent problem without a simple solution.

 

[Chris Shepherd]

times within the span of several lines. The onus is therefore on the
programmer to:ensure that the property *is* cheap. This may not be
possible in all cases however (e.g., the value has to be dynamically
calculated or retrieved on each call), or you're otherwise forced to cache
the value in a member variable. In the former case you may be forced to

I don't think it's necessary to always make a property cheap. Sometimes
it makes sense in cases where it is possible to be cheap, but you know
that setting this property is usually followed by an action. An example
of this would be setting the DataSource on a DataGridView. If I set the
DataSource property, it does a lot of things like raising an event
(DataSourceChanged), creating its columns, populating them with data,
and so on. This is because it makes perfect sense, even though it's not
necessary to do this. It could just have a method like
ActivateDataSource() or something which would perform all these steps,
but 99 times out of 100, when you set the DataSource you'd be calling
this method anyway.

That's just one example, but there are several similar ones in the
framework. For the most part, I'd use the framework as a guideline on
issues such as this. URI might be a good comparable example -- anyone
know how it handles things internally?

Chris.

 

[John Duval]

That sounds like a premature optimisation to me - I'd wait until there
was any indication that it was a problem before doing that. In
particular, if you're finding a directory name you're probably going
to do some IO, and that's bound to dwarf the cost of Path.Combine.

Jon

True, it's very unlikely to have any noticable performance impact. I
agree that it's best to take the "optimize when necessary" approach,
and prefer to stick with the simpler code unless you can measure a
performance difference.

 

[Larry Smith]

I don't think it's necessary to always make a property cheap. Sometimes it

makes sense in cases where it is possible to be cheap, but you know that
setting this property is usually followed by an action. An example of this
would be setting the DataSource on a DataGridView. If I set the DataSource
property, it does a lot of things like raising an event
(DataSourceChanged), creating its columns, populating them with data, and
so on. This is because it makes perfect sense, even though it's not
necessary to do this. It could just have a method like
ActivateDataSource() or something which would perform all these steps, but
99 times out of 100, when you set the DataSource you'd be calling this
method anyway.

That's just one example, but there are several similar ones in the
framework. For the most part, I'd use the framework as a guideline on
issues such as this. URI might be a good comparable example -- anyone know
how it handles things internally?

I agree that very few rules in life are absolute. A property may need to be
infrequently get/set (sometimes just once) and that's it. It may also be
self-evident that some properties carry a lot of overhead due to the nature
of the property itself. In practice however most properties typically
represent some internal state that programmers feel carefree about
getting/setting often or getting/setting multiple times in a very short
timespan ("getting" being more typical). This is really where the situation
can become a potential problem. Someone can perform a lot of looping for
instance and either directly call some expensive property or perhaps
indirectly via some other function. You can then end up with (potentially
serious) performance problems and nobody will have a clue why. The program
will just run sluggishly and this may not even occur all the time. It may
not even become clear until it's actually released. By that time however
it's usually very difficult as well as expensive for most organizations to
re-examine their project in order to find possible bottlenecks. I think
there's a built-in tendency for programmers to assume that normal functions
should be called sparingly but properties can be called as frequently as you
want. You do so at your own peril however.

 

[Peter Duniho]

On Oct 24, 1:29 pm, (e-mail address removed) wrote:



Just be aware that your terminology isn't universally known -
"relative" and "absolute" are more common ways of describing the
difference.

Or alternatively, if "Folder" is always going to be the very last
directory name in the path, a name indicating that could be at least as
useful as describing it as "relative". For example, TestFolderName, or
something similar making it clear that the "folder" property is the name
of a single object.

Pete