Class function help

[JackC]

Hi,

I have written the following function to retrieve a class instance
from a vector, if found it returns the object.

CServer CServerControl::GetServer(string alias)
{

for(vector<CServer>::size_type i = 0; i < servers.size(); ++i)
{
if(servers.GetServerAlias() == alias)
{
// Return the server
return servers;
}
}

// Return error value here

}


Question is, how do i return a bad value if the alias is not found in
the vector? It wont let me return a NULL reference, maybe i am
approaching this incorrectly? Any tips on a better solution?

Thanks
Jack

 

[Alf P. Steinbach]

* JackC:

I have written the following function to retrieve a class instance
from a vector, if found it returns the object.

CServer CServerControl::GetServer(string alias)
{

for(vector<CServer>::size_type i = 0; i < servers.size(); ++i)
{
if(servers.GetServerAlias() == alias)
{
// Return the server
return servers;
}
}

// Return error value here

}


Question is, how do i return a bad value if the alias is not found in
the vector? It wont let me return a NULL reference, maybe i am
approaching this incorrectly? Any tips on a better solution?

There are number of approaches, e.g.

1. Return an object that can be logically empty or contain a
CSever object, with suitable access protocol to avoid accessing
a non-existent object.

2. Return a pointer or iterator.

3. Throw an exception (and possibly also provide a way to check
beforehand if the sought for vector element exists).

The said, do you really mean to return a CServer by value, and do you
really mean to pass the string argument by value?

And (but this is just style issue) are all those prefixes, "C" and
"Get", really helping anything? For the string argument I'd write
"std::string", because there the prefix is practically useful, but there
you omit it. I'd never introduce the "C" and "Get" prefixes because for
me they don't help with anything, they're just in the way of clarity.

Cheers, & hth.,

- Alf

 

[JackC]

* JackC:

I have written the following function to retrieve a class instance
from a vector, if found it returns the object.

CServer CServerControl::GetServer(string alias)
{

for(vector<CServer>::size_type i = 0; i < servers.size(); ++i)
{
if(servers.GetServerAlias() == alias)
{
// Return the server
return servers;
}
}

// Return error value here

Question is, how do i return a bad value if the alias is not found in
the vector? It wont let me return a NULL reference, maybe i am
approaching this incorrectly? Any tips on a better solution?

There are number of approaches, e.g.

1. Return an object that can be logically empty or contain a
CSever object, with suitable access protocol to avoid accessing
a non-existent object.

2. Return a pointer or iterator.

3. Throw an exception (and possibly also provide a way to check
beforehand if the sought for vector element exists).

The said, do you really mean to return a CServer by value, and do you
really mean to pass the string argument by value?

And (but this is just style issue) are all those prefixes, "C" and
"Get", really helping anything? For the string argument I'd write
"std::string", because there the prefix is practically useful, but there
you omit it. I'd never introduce the "C" and "Get" prefixes because for
me they don't help with anything, they're just in the way of clarity.

Cheers, & hth.,

- Alf

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Thanks for the help, I suppose the best way would be to return a
pointer to it, then i can return a null pointer if it is not found.

When you say did i mean to pass the alias string by value, well i did
because i have never really been taught any better. Is this just
considered bad programming in terms of using unnecessary resources
when i could use a pointer instead?

And as for the style, again its what i picked up from when i started
programming when I was about 13, maybe i should try and get out of the
habit though.
You say using Get is bad, I have created Get/Set functions and kept
the variable public, is this again a wrong approach? Would it be
better design to just make them public and scrap the get/set
functions.

Thanks again for any advice.
Jack

 

[Alf P. Steinbach]

* JackC:

* JackC:

I have written the following function to retrieve a class instance
from a vector, if found it returns the object.
CServer CServerControl::GetServer(string alias)
{
for(vector<CServer>::size_type i = 0; i < servers.size(); ++i)
{
if(servers.GetServerAlias() == alias)
{
// Return the server
return servers;
}
}
// Return error value here
}
Question is, how do i return a bad value if the alias is not found in
the vector? It wont let me return a NULL reference, maybe i am
approaching this incorrectly? Any tips on a better solution?

There are number of approaches, e.g.

1. Return an object that can be logically empty or contain a
CSever object, with suitable access protocol to avoid accessing
a non-existent object.

2. Return a pointer or iterator.

3. Throw an exception (and possibly also provide a way to check
beforehand if the sought for vector element exists).

The said, do you really mean to return a CServer by value, and do you
really mean to pass the string argument by value?

And (but this is just style issue) are all those prefixes, "C" and
"Get", really helping anything? For the string argument I'd write
"std::string", because there the prefix is practically useful, but there
you omit it. I'd never introduce the "C" and "Get" prefixes because for
me they don't help with anything, they're just in the way of clarity.

Cheers, & hth.,

- Alf

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Thanks for the help, I suppose the best way would be to return a
pointer to it, then i can return a null pointer if it is not found.

When you say did i mean to pass the alias string by value, well i did
because i have never really been taught any better. Is this just
considered bad programming in terms of using unnecessary resources
when i could use a pointer instead?

Using a pointer is considered even worse style.

Use

std::string const& alias

And as for the style, again its what i picked up from when i started
programming when I was about 13, maybe i should try and get out of the
habit though.
You say using Get is bad, I have created Get/Set functions and kept
the variable public, is this again a wrong approach? Would it be
better design to just make them public and scrap the get/set
functions.

Keeping the member variable public is generally ungood, but in some
cases (e.g. a simple Point class) it can be good. So it depends very
much on context. And so does the use of getters/setters, although too
many getters/setters generally indicate unclean design, e.g. instead of
"x()" and "setX()", consider what that x value is used for, then provide
the higher level functionality (and incidentally thereby removing a host
of possible failure modes and bug entry vectors).

Cheers, & hth.,

- Alf

 

[James Kanze]

* JackC:

I have written the following function to retrieve a class instance
from a vector, if found it returns the object.
CServer CServerControl::GetServer(string alias)
{
for(vector<CServer>::size_type i = 0; i < servers.size(); ++i)
{
if(servers.GetServerAlias() == alias)
{
// Return the server
return servers;
}
}
// Return error value here
}
Question is, how do i return a bad value if the alias is not
found in the vector? It wont let me return a NULL reference,
maybe i am approaching this incorrectly? Any tips on a
better solution?

There are number of approaches, e.g.

1. Return an object that can be logically empty or contain a
CSever object, with suitable access protocol to avoid accessing
a non-existent object.

2. Return a pointer or iterator.

3. Throw an exception (and possibly also provide a way to check
beforehand if the sought for vector element exists).

The said, do you really mean to return a CServer by value, and
do you really mean to pass the string argument by value?

Agreed. I'd probably return a pointer, since pointers already
have a special value (null) which can be used for signaling
errors. (And of course, I'd definitely pass an std::string
argument by const reference.)

And (but this is just style issue) are all those prefixes, "C"
and "Get", really helping anything?

Isn't the C prefix reserved by Microsoft for classes in MFC? I
would definitly avoid it, if only for that reason. (The pros
and contras of the Get... are somewhat more complex. In this
particular case, I'd probably use it, but I'd have to see more
of the context to be sure.)

For the string argument I'd write "std::string", because there
the prefix is practically useful, but there you omit it. I'd
never introduce the "C" and "Get" prefixes because for me they
don't help with anything, they're just in the way of clarity.

The C tells you that the class is part of MFC. The Get may be
part of a larger convention, in which function names are verbs,
unqualified nouns are types, and qualified nouns are variables.
The larger convention is quite useful, if used with
intelligence.

 

[James Kanze]

On 30 Nov, 22:42, "Alf P. Steinbach" <[email protected]> wrote:

Thanks for the help, I suppose the best way would be to return
a pointer to it, then i can return a null pointer if it is not
found.

That's what I'd do. I think, at least; I'd have to have more
context to be sure.

When you say did i mean to pass the alias string by value,
well i did because i have never really been taught any better.
Is this just considered bad programming in terms of using
unnecessary resources when i could use a pointer instead?

It's not really wrong, but most people have the automatique
habit of passing class types by const reference.

And as for the style, again its what i picked up from when i
started programming when I was about 13, maybe i should try
and get out of the habit though.

You say using Get is bad, I have created Get/Set functions and
kept the variable public, is this again a wrong approach?

Again, it depends. Usually, you don't want public or protected
data members (although there are exceptions), and I'm pretty
sure that Alf isn't recommending that. If the "value" is a
simple attribute of the object, then it is conceptually a
"variable"; the function name reflects this, and is a qualified
noun (e.g. status)---it is often overloaded with a const member
which returns the value, and a non-const member which takes a
new value as an argument. (Note, however, that too much of this
is generally a sign that you haven't really understood
encapsulation. Most "attributes" of a class probably shouldn't
be visible as attributes from the outside. But again, there are
exceptions.)

In the case in question, however, I'm not sure that we are
dealing with an attribute. If the function is an explicit
look-up, then using get as part of the name is fine. Some
people also recommend it for attributes. I don't, but that is a
style issue. If the function represents some action, however,
and not just state, then the name should be a verb.

Of course, as verbs go, "get" is not particularly precise.
Depending on the context, perhaps "find" or "create" would be
better. (In this case, "find", of course, rather than
"create".) Or something application specific, even more
precise.

 

[Alf P. Steinbach]

* James Kanze:

* JackC:

I have written the following function to retrieve a class instance
from a vector, if found it returns the object.
CServer CServerControl::GetServer(string alias)
{
for(vector<CServer>::size_type i = 0; i < servers.size(); ++i)
{
if(servers.GetServerAlias() == alias)
{
// Return the server
return servers;
}
}
// Return error value here
}
Question is, how do i return a bad value if the alias is not
found in the vector? It wont let me return a NULL reference,
maybe i am approaching this incorrectly? Any tips on a
better solution?

There are number of approaches, e.g.

1. Return an object that can be logically empty or contain a
CSever object, with suitable access protocol to avoid accessing
a non-existent object.

2. Return a pointer or iterator.

3. Throw an exception (and possibly also provide a way to check
beforehand if the sought for vector element exists).

4. Return a Fallible< Server >.

I think that was included in point 1.

Btw., not everyone have read Barton & Nackmannnnnnnn.

E.g., it's still on my "to steal" list.


Cheers,

- Alf