Tek-Tips is the largest IT community on the Internet today!

Members share and learn making Tek-Tips Forums the best source of peer-reviewed technical information on the Internet!

Register Log in
  • Congratulations Andrzejek on being selected by the Tek-Tips community for having the most helpful posts in the forums last week. Way to Go!

How to set collection from inside constructor 1

Status
Not open for further replies.
tshad

tshad

Programmer
Jul 15, 2004
386
US
I have a class and a list<class> that I want to set up in the constructor. I want to fill the collection when I instantiate the object.

Code: 
    public class DotNetType
    {
        public string DatabaseDataType { get; set; }
        public string DotNetDataType { get; set; }

    }

    public class DotNetTypeCollection : List<DotNetType>
    {
        public DotNetTypeCollection()
        {
            DbObject dbo = new DbObject();
            DataSet ds = null;
            ds = dbo.GetDataSet("dbGen.GetDotNetDataTypes");

            this = ds.Tables[0].AsEnumerable().Select(row => new DotNetType
                            {
                                DotNetDataType = row.Field<string>("DotNetDataType"),
                                DatabaseDataType = row.Field<string>("DatabaseDataType")
                            }).ToList();
        }
    }

I get an error that "this" is readonly.

Can this be done?

Thanks,

Tom
 
Sort by date Sort by votes
No, you can't overwrite the current object with a new one from that object. In this case, you'd be better off just doing something like this:

Code: 
[COLOR=#0000FF]public[/color] [COLOR=#0000FF]class[/color] [COLOR=#2B91AF]DotNetType[/color]
{
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]string[/color] DatabaseDataType { [COLOR=#0000FF]get[/color]; [COLOR=#0000FF]set[/color]; }
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]string[/color] DotNetDataType { [COLOR=#0000FF]get[/color]; [COLOR=#0000FF]set[/color]; }
 
}
 
[COLOR=#0000FF]public[/color] [COLOR=#0000FF]class[/color] [COLOR=#2B91AF]DotNetTypeCollection[/color] : [COLOR=#2B91AF]List[/color]&lt;[COLOR=#2B91AF]DotNetType[/color]&gt;
{
    [COLOR=#0000FF]public[/color] DotNetTypeCollection() : [COLOR=#0000FF]base[/color]()
    {
        [COLOR=#2B91AF]DbObject[/color] dbo = [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]DbObject[/color]();
        [COLOR=#2B91AF]DataSet[/color] ds = [COLOR=#0000FF]null[/color];
        ds = dbo.GetDataSet([COLOR=#A31515]&quot;dbGen.GetDotNetDataTypes&quot;[/color]);
 
        [COLOR=#0000FF]this[/color].AddRange(ds.Tables&#91;0&#93;.AsEnumerable().Select(row =&gt; [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]DotNetType[/color]
        {
            DotNetDataType = row.Field&lt;[COLOR=#0000FF]string[/color]&gt;([COLOR=#A31515]&quot;DotNetDataType&quot;[/color]),
            DatabaseDataType = row.Field&lt;[COLOR=#0000FF]string[/color]&gt;([COLOR=#A31515]&quot;DatabaseDataType&quot;[/color])
        }).ToList());
    }
}


For future reference though, the general 'good practice' for doing something like this, where you need to basically make a constructor that returns a new instance of the object, is by making a static method instead. Keep in mind however that your example isn't really a good use case for doing it this way; a normal constructor works just fine for you.

Code: 
[COLOR=#0000FF]public[/color] [COLOR=#0000FF]class[/color] [COLOR=#2B91AF]DotNetType[/color]
{
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]string[/color] DatabaseDataType { [COLOR=#0000FF]get[/color]; [COLOR=#0000FF]set[/color]; }
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]string[/color] DotNetDataType { [COLOR=#0000FF]get[/color]; [COLOR=#0000FF]set[/color]; }
 
}
 
[COLOR=#0000FF]public[/color] [COLOR=#0000FF]class[/color] [COLOR=#2B91AF]DotNetTypeCollection[/color] : [COLOR=#2B91AF]List[/color]&lt;[COLOR=#2B91AF]DotNetType[/color]&gt;
{
    [COLOR=#0000FF]private[/color] DotNetTypeCollection() : [COLOR=#0000FF]base[/color]() { }
 
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]DotNetTypeCollection[/color] CreateDotNetTypeCollection()
    {
        [COLOR=#2B91AF]DbObject[/color] dbo = [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]DbObject[/color]();
        [COLOR=#2B91AF]DataSet[/color] ds = [COLOR=#0000FF]null[/color];
        ds = dbo.GetDataSet([COLOR=#A31515]&quot;dbGen.GetDotNetDataTypes&quot;[/color]);
 
        [COLOR=#2B91AF]DotNetTypeCollection[/color] collection = [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]DotNetTypeCollection[/color]();
 
        collection.AddRange(ds.Tables&#91;0&#93;.AsEnumerable().Select(row =&gt; [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]DotNetType[/color]
        {
            DotNetDataType = row.Field&lt;[COLOR=#0000FF]string[/color]&gt;([COLOR=#A31515]&quot;DotNetDataType&quot;[/color]),
            DatabaseDataType = row.Field&lt;[COLOR=#0000FF]string[/color]&gt;([COLOR=#A31515]&quot;DatabaseDataType&quot;[/color])
        }).ToList());
 
        [COLOR=#0000FF]return[/color] collection;
    }
}
 
Upvote 0 Downvote
That did exactly what I wanted. I couldn't figure out what to use "this" with to add the collection. "this.Addrange" worked perfectly.

Now you said the normal good practice was to do it the 2nd way with the static method, but that my example wasn't a good case for doing it this way. Why not? I prefer the 1st way, but when would I do it the 1st way and when the 2nd?

I had a similar case that I did which also did it the 1st way, but I couldn't figure out how to get this one to work that way:

Code: 
    public class StatementCauseCollection : List<StatementClause>
    {
        public StatementCauseCollection(int objectID, string objectType)
        {
            SetUpStatementClauseList(objectID, objectType);
        }
        
        private StatementCauseCollection()
        {
        }

        public void SetUpStatementClauseList(int objectID, string objectType)
        {
            if (objectType == "U" || objectType == "V")
            {
                this.Add(new StatementClause(objectID, StatementClauseTypes.Where));
                this.Add(new StatementClause(objectID, StatementClauseTypes.OrderBy));
                this.Add(new StatementClause(objectID, StatementClauseTypes.Select));
                if (objectType == "U")
                {
                    this.Add(new StatementClause(objectID, StatementClauseTypes.Insert));
                    this.Add(new StatementClause(objectID, StatementClauseTypes.Update));
                    this.Add(new StatementClause(objectID, StatementClauseTypes.Delete));
                }
            }
        }
    }

Thanks,

Tom
 
Upvote 0 Downvote
Jon Skeet summarizes reasons for doing and not doing this quite nicely, at least in my opinion.


In your case, there isn't really a good reason to go with a factory method. I typically do that in scenarios like... when working with Micros, the communication are all done over TCP, using Message Framing. I have everything I need all wrapped up in a class library. I don't want someone using the Message class to be able to construct a message themselves; they must provide the ASCII string received from the TCP socket. If it isn't a valid format, I want to be able to make the instance null, which makes for a very easy check. So in this case, I did something like this:

Code: 
[COLOR=#0000FF]namespace[/color] ConsoleApplication1
{
    [COLOR=#0000FF]class[/color] [COLOR=#2B91AF]MicrosMessage[/color]
    {
        [COLOR=#0000FF]private[/color] MicrosMessage() { }
 
        [COLOR=#0000FF]public[/color] [COLOR=#0000FF]int[/color] WorkstationNumber { [COLOR=#0000FF]get[/color]; [COLOR=#0000FF]private[/color] [COLOR=#0000FF]set[/color]; }
 
        [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]MicrosMessage[/color] MessageFromString([COLOR=#0000FF]string[/color] rawData)
        {
            [COLOR=#0000FF]if[/color] (!IsValidMessage(rawData))
                [COLOR=#0000FF]return[/color] [COLOR=#0000FF]null[/color];
            [COLOR=#2B91AF]MicrosMessage[/color] mm = [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]MicrosMessage[/color]();
            mm.WorkstationNumber = 10;
            [COLOR=#008000]//do work here....[/color]
            [COLOR=#0000FF]return[/color] mm;
        }
 
        [COLOR=#0000FF]private[/color] [COLOR=#0000FF]static[/color] [COLOR=#0000FF]bool[/color] IsValidMessage([COLOR=#0000FF]string[/color] data)
        {
            [COLOR=#0000FF]return[/color] [COLOR=#0000FF]true[/color];
        }
    }
}

Notice I made the empty constructor private - that means the ONLY way to create an instance of MicrosMessage is by calling my factory method. I also made my one attribute with a public getter, and private setter, which means that the values can ONLY be set from within the MicrosMessage class, which means they can't accidentally overwrite the information anywhere. I could do the same by only exposing a single constructor that requires a string, but now I have to also factor in doing things like creating something like an IsValid attribute to check against - the new constructor ALWAYS creates a new instances, excepting when an exception gets thrown.

This could be a good use case for you if you need a way to check for failures, for example, but it doesn't look like that is the case here? There is also, of course, design preference. Some people prefer sticking to the new Object() paradigm, and other like using the static methods because it can make things clearer.
 
Upvote 0 Downvote
I don't think it's a good use-case because typically you'd only do something like that when you need to pass it some information, and get a new object from it. It is usually something you'd do when you want to prevent creating a new instance of the object with the default settings, and enforce creating the object is specific ways, though technically you can do that with normal constructors, it would get VERY confusing trying to create those overloads:

Code: 
[COLOR=#0000FF]public[/color] [COLOR=#0000FF]class[/color] [COLOR=#2B91AF]MicrosDatabase[/color] : [COLOR=#2B91AF]IDisposable[/color]
{
    [COLOR=#0000FF]private[/color] [COLOR=#2B91AF]IDbConnection[/color] _IDbConnection;
 
    [COLOR=#0000FF]private[/color] MicrosDatabase() { }
 
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]MicrosDatabase[/color] OdbcDbConnection()
    {
        [COLOR=#0000FF]return[/color] [COLOR=#2B91AF]MicrosDatabase[/color].OdbcDbConnection([COLOR=#A31515]&quot;custom&quot;[/color], [COLOR=#A31515]&quot;custom&quot;[/color], [COLOR=#A31515]&quot;micros&quot;[/color]);
    }
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]MicrosDatabase[/color] OdbcDbConnection([COLOR=#0000FF]string[/color] Username, [COLOR=#0000FF]string[/color] Password)
    {
        [COLOR=#0000FF]return[/color] [COLOR=#2B91AF]MicrosDatabase[/color].OdbcDbConnection(Username, Password, [COLOR=#A31515]&quot;micros&quot;[/color]);
    }
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]MicrosDatabase[/color] OdbcDbConnection([COLOR=#0000FF]string[/color] Username, [COLOR=#0000FF]string[/color] Password, [COLOR=#0000FF]string[/color] DSN)
    {
        [COLOR=#0000FF]return[/color] [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]MicrosDatabase[/color]()
        {
            _IDbConnection = [COLOR=#0000FF]new[/color] System.Data.Odbc.[COLOR=#2B91AF]OdbcConnection[/color]([COLOR=#0000FF]string[/color].Format([COLOR=#A31515]&quot;DSN={0};UID={1};PWD={2}&quot;[/color], DSN, Username, Password))
        };
    }
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]MicrosDatabase[/color] OleDbConnection([COLOR=#0000FF]string[/color] Username, [COLOR=#0000FF]string[/color] Password)
    {
        [COLOR=#0000FF]return[/color] [COLOR=#2B91AF]MicrosDatabase[/color].OleDbConnection([COLOR=#2B91AF]MicrosOleProvider[/color].RES5, [COLOR=#2B91AF]Environment[/color].MachineName, Username, Password, [COLOR=#A31515]&quot;127.0.0.1&quot;[/color]);
    }
    [COLOR=#0000FF]public[/color] [COLOR=#0000FF]static[/color] [COLOR=#2B91AF]MicrosDatabase[/color] OleDbConnection([COLOR=#2B91AF]MicrosOleProvider[/color] Provider, [COLOR=#0000FF]string[/color] ServerName, [COLOR=#0000FF]string[/color] Username, [COLOR=#0000FF]string[/color] Password, [COLOR=#0000FF]string[/color] IPAddress)
    {
        [COLOR=#0000FF]return[/color] [COLOR=#0000FF]new[/color] [COLOR=#2B91AF]MicrosDatabase[/color]()
        {
            _IDbConnection = [COLOR=#0000FF]new[/color] System.Data.OleDb.[COLOR=#2B91AF]OleDbConnection[/color](
                [COLOR=#0000FF]string[/color].Format([COLOR=#A31515]&quot;Provider={0};EngineName=sql{1};UID={2};PWD={3};Links=TCPIP(Host={4})&quot;[/color],
                    Provider, ServerName, Username, Password, IPAddress))
        };
    }

So as you can see, I'm using a generic IDbConnection; once I get it constructed, it doesn't matter how I got connected. I can run queries and stored procedures and what not exactly the same way; the only time I need to be concerned with the connection method is when the user is creating it, but I need a way to have what almost amounts to typed constructors. It would be very difficult to create proper construction overloads for this because of the sheer amount of overlap there is. This is only a slice from this object, because this class handles connection to two different database types using several different methods for both.
 
Upvote 0 Downvote
Status
Not open for further replies.

Similar threads

xarzu
  • Locked
  • Question
How do I set up a new computer to run an executable I have built with Microsoft Visual Studio?
Replies
1
Views
903
gbaughma
gbaughma
xarzu
  • Locked
  • Question
How do I connect my existing Visual Studio 2022 project to an existing github repository?
Replies
0
Views
597
xarzu
xarzu
hoangluc19
  • Locked
  • Question
Load data from database to Datagridview C#
Replies
0
Views
667
hoangluc19
hoangluc19
ahmedsa2018
  • Locked
  • Question
How to get difference between two array without using linq or Set operator using csharp ?
Replies
8
Views
618
mikrom
mikrom
xarzu
  • Locked
  • Question
What is the proper way to declare a Microsoft class to use in my code. The quick and dirty way I am doing it here, I know, is not correct
Replies
0
Views
872
xarzu
xarzu

Part and Inventory Search

Sponsor

Back
Top