[erburrell]

All,

I am trying to learn the best way to code my classes, and was wondering if I could get a bit of a critique on the following? I have a class that provides a basic structure, and a separate class that provides a data repository that does the database interface. Have I done it well, or are there better ways to do this.

As a additional note, I am using an Access Database with a dataset for the interface.

Thanks,

Ed.

My Code:

public class DailyLog
    {
        public int Id { get; set; }
        public DateTime LogDate { get; set; }
        public double PurchasedPowerMeter { get; set; }
        public double NetGenMeter { get; set; }
        public double NetGasMeter { get; set; }
        public double LimestoneBeltTotalizer { get; set; }
        public double C1BeltTotalizer { get; set; }
        public double C5ABeltTotalizer { get; set; }
        public double C5BBeltTotalizer { get; set; }
        public double FeedTotalizer11_100 { get; set; }
        public double FeedTotalizer11_200 { get; set; }
        public double FeedTotalizer11_300 { get; set; }
        public double FeedTotalizer11_400 { get; set; }
        public double FeedTotalizer12_100 { get; set; }
        public double FeedTotalizer12_200 { get; set; }
        public double FeedTotalizer12_300 { get; set; }
        public double FeedTotalizer12_400 { get; set; }
        public string LigniteSampleNo { get; set; }
        public double LigniteSampleWeight { get; set; }
        public double C5ABeltZeroError { get; set; }
        public double C5BBeltZeroError { get; set; }
        public double WaterLoss { get; set; }
        public double H2LeakRate { get; set; }
        public double HoursOn { get; set; }
        public double Boiler1HoursOn { get; set; }
        public double Boiler2HoursOn { get; set; }
        public string Comments { get; set; }

        public DailyLog()
        {
            this.Id = 0;
            this.LogDate = DateTime.MinValue;
            this.PurchasedPowerMeter = 0.0;
            this.NetGenMeter = 0.0;
            this.NetGasMeter = 0.0;
            this.LimestoneBeltTotalizer = 0.0;
            this.C1BeltTotalizer = 0.0;
            this.C5ABeltTotalizer = 0.0;
            this.C5BBeltTotalizer = 0.0;
            this.FeedTotalizer11_100 = 0.0;
            this.FeedTotalizer11_200 = 0.0;
            this.FeedTotalizer11_300 = 0.0;
            this.FeedTotalizer11_400 = 0.0;
            this.FeedTotalizer12_100 = 0.0;
            this.FeedTotalizer12_200 = 0.0;
            this.FeedTotalizer12_300 = 0.0;
            this.FeedTotalizer12_400 = 0.0;
            this.LigniteSampleNo = string.Empty;
            this.LigniteSampleWeight = 0.0;
            this.C5ABeltZeroError = 0.0;
            this.C5BBeltZeroError = 0.0;
            this.WaterLoss = 0.0;
            this.H2LeakRate = 0.0;
            this.HoursOn = 0.0;
            this.Boiler1HoursOn = 0.0;
            this.Boiler2HoursOn = 0.0;
            this.Comments = string.Empty;
        }

        public DailyLog(DailyLogData.DailyLogRow dailyLogRow)
        {
            this.Id = dailyLogRow.Id;
            this.LogDate = dailyLogRow.LogDate;
            this.PurchasedPowerMeter = dailyLogRow.PurchasedPowerMeter;
            this.NetGenMeter = dailyLogRow.NetGenMeter;
            this.NetGasMeter = dailyLogRow.NetGenMeter;
            this.LimestoneBeltTotalizer = dailyLogRow.LimestoneBeltTotalizer;
            this.C1BeltTotalizer = dailyLogRow.C1BeltTotalizer;
            this.C5ABeltTotalizer = dailyLogRow.C5ABeltTotalizer;
            this.C5BBeltTotalizer = dailyLogRow.C5BBeltTotalizer;
            this.FeedTotalizer11_100 = dailyLogRow.FeedTotalizer_11_100;
            this.FeedTotalizer11_200 = dailyLogRow.FeedTotalizer_11_200;
            this.FeedTotalizer11_300 = dailyLogRow.FeedTotalizer_11_300;
            this.FeedTotalizer11_400 = dailyLogRow.FeedTotalizer_11_400;
            this.FeedTotalizer12_100 = dailyLogRow.FeedTotalizer_12_100;
            this.FeedTotalizer12_200 = dailyLogRow.FeedTotalizer_12_200;
            this.FeedTotalizer12_300 = dailyLogRow.FeedTotalizer_12_300;
            this.FeedTotalizer12_400 = dailyLogRow.FeedTotalizer_12_400;
            this.LigniteSampleNo = dailyLogRow.LigniteSampleNo;
            this.LigniteSampleWeight = dailyLogRow.LigniteSampleWeight;
            this.C5ABeltZeroError = dailyLogRow.C5ABeltZeroError;
            this.C5BBeltZeroError = dailyLogRow.C5BBeltZeroError;
            this.WaterLoss = dailyLogRow.WaterLoss;
            this.H2LeakRate = dailyLogRow.H2LeakRate;
            this.HoursOn = dailyLogRow.HoursOn;
            this.Boiler1HoursOn = dailyLogRow.Boiler1HoursOn;
            this.Boiler2HoursOn = dailyLogRow.Boiler2HoursOn;
            this.Comments = dailyLogRow.Comments;
        }
    }

    public class DailyLogrepository
    {
        private DailyLogTableAdapter _adapter = null;
        protected DailyLogTableAdapter Adapter
        {
            get
            {
                if (_adapter == null)
                    _adapter = new DailyLogTableAdapter();
                return _adapter;
            }
        }

        public IList<DailyLog> getAllDailyLogs()
        {
            IList<DailyLog> dailyLogList = new List<DailyLog>();
            DailyLogData.DailyLogDataTable allLogs = Adapter.GetDailyLogs();
            foreach (DailyLogData.DailyLogRow logrow in allLogs)
            {
                dailyLogList.Add(new DailyLog(logrow));
            }

            return dailyLogList;
        }

        public IList<DailyLog> GetDailyLogsByDateRange(DateTime startDate, DateTime endDate)
        {
            IList<DailyLog> dailyLogList = new List<DailyLog>();
            DailyLogData.DailyLogDataTable allLogs = Adapter.GetDailyLogsByDateRange(startDate, endDate);
            foreach (DailyLogData.DailyLogRow logrow in allLogs)
            {
                dailyLogList.Add(new DailyLog(logrow));
            }

            return dailyLogList;
        }

        public IList<DailyLog> GetDailyLogsByIdRange(int startId, int endId)
        {
            IList<DailyLog> dailyLogList = new List<DailyLog>();
            DailyLogData.DailyLogDataTable allLogs = Adapter.GetDailyLogsByIdRange(startId, endId);
            foreach (DailyLogData.DailyLogRow logrow in allLogs)
            {
                dailyLogList.Add(new DailyLog(logrow));
            }

            return dailyLogList;
        }

        public IList<DailyLog> GetLast100DailyLogs()
        {
            IList<DailyLog> dailyLogList = new List<DailyLog>();
            DailyLogData.DailyLogDataTable allLogs = Adapter.GetDailyLogs();
            if (allLogs.Count == 0)
                return dailyLogList;
            int firstId = allLogs[0].Id; // Since the logs are given in descending date range, the first log should be the last one given and have the highest Id.
            DailyLogData.DailyLogDataTable allLast100Logs = Adapter.GetDailyLogsByIdRange(firstId, firstId-100);
            foreach (DailyLogData.DailyLogRow logrow in allLogs)
            {
                dailyLogList.Add(new DailyLog(logrow));
            }

            return dailyLogList;
        }

        public DailyLog GetDailyLogById(int id)
        {
            DailyLogData.DailyLogDataTable logs = Adapter.GetDailyLogsById(id);
            if (logs.Count == 0)
                return new DailyLog();

            return new DailyLog(logs[0]);
        }

        public bool AddDailyLog(DailyLog newLog)
        {
            DailyLogData.DailyLogDataTable logTable = new DailyLogData.DailyLogDataTable();
            DailyLogData.DailyLogRow logRow = logTable.NewDailyLogRow();

            logRow.LogDate = newLog.LogDate;
            logRow.PurchasedPowerMeter = newLog.PurchasedPowerMeter;
            logRow.NetGenMeter = newLog.NetGenMeter;
            logRow.NetGenMeter = newLog.NetGasMeter;
            logRow.LimestoneBeltTotalizer = newLog.LimestoneBeltTotalizer;
            logRow.C1BeltTotalizer = newLog.C1BeltTotalizer;
            logRow.C5ABeltTotalizer = newLog.C5ABeltTotalizer;
            logRow.C5BBeltTotalizer = newLog.C5BBeltTotalizer;
            logRow.FeedTotalizer_11_100 = newLog.FeedTotalizer11_100;
            logRow.FeedTotalizer_11_200 = newLog.FeedTotalizer11_200;
            logRow.FeedTotalizer_11_300 = newLog.FeedTotalizer11_300;
            logRow.FeedTotalizer_11_400 = newLog.FeedTotalizer11_400;
            logRow.FeedTotalizer_12_100 = newLog.FeedTotalizer12_100;
            logRow.FeedTotalizer_12_200 = newLog.FeedTotalizer12_200;
            logRow.FeedTotalizer_12_300 = newLog.FeedTotalizer12_300;
            logRow.FeedTotalizer_12_400 = newLog.FeedTotalizer12_400;
            logRow.LigniteSampleNo = newLog.LigniteSampleNo;
            logRow.LigniteSampleWeight = newLog.LigniteSampleWeight;
            logRow.C5ABeltZeroError = newLog.C5ABeltZeroError;
            logRow.C5BBeltZeroError = newLog.C5BBeltZeroError;
            logRow.WaterLoss = newLog.WaterLoss;
            logRow.H2LeakRate = newLog.H2LeakRate;
            logRow.HoursOn = newLog.HoursOn;
            logRow.Boiler1HoursOn = newLog.Boiler1HoursOn;
            logRow.Boiler2HoursOn = newLog.Boiler2HoursOn;
            logRow.Comments = newLog.Comments;

            logTable.AddDailyLogRow(logRow);

            int rowsAffected = Adapter.Update(logTable);

            return rowsAffected == 1;
        }

        public bool UpateDailyLog(DailyLog dailyLog)
        {
            DailyLogData.DailyLogDataTable logTable = Adapter.GetDailyLogsById(dailyLog.Id);
            if (logTable.Count == 0)
                return false;

            DailyLogData.DailyLogRow logRow = logTable[0];

            logRow.Id = dailyLog.Id;
            logRow.LogDate = dailyLog.LogDate;
            logRow.PurchasedPowerMeter = dailyLog.PurchasedPowerMeter;
            logRow.NetGenMeter = dailyLog.NetGenMeter;
            logRow.NetGenMeter = dailyLog.NetGasMeter;
            logRow.LimestoneBeltTotalizer = dailyLog.LimestoneBeltTotalizer;
            logRow.C1BeltTotalizer = dailyLog.C1BeltTotalizer;
            logRow.C5ABeltTotalizer = dailyLog.C5ABeltTotalizer;
            logRow.C5BBeltTotalizer = dailyLog.C5BBeltTotalizer;
            logRow.FeedTotalizer_11_100 = dailyLog.FeedTotalizer11_100;
            logRow.FeedTotalizer_11_200 = dailyLog.FeedTotalizer11_200;
            logRow.FeedTotalizer_11_300 = dailyLog.FeedTotalizer11_300;
            logRow.FeedTotalizer_11_400 = dailyLog.FeedTotalizer11_400;
            logRow.FeedTotalizer_12_100 = dailyLog.FeedTotalizer12_100;
            logRow.FeedTotalizer_12_200 = dailyLog.FeedTotalizer12_200;
            logRow.FeedTotalizer_12_300 = dailyLog.FeedTotalizer12_300;
            logRow.FeedTotalizer_12_400 = dailyLog.FeedTotalizer12_400;
            logRow.LigniteSampleNo = dailyLog.LigniteSampleNo;
            logRow.LigniteSampleWeight = dailyLog.LigniteSampleWeight;
            logRow.C5ABeltZeroError = dailyLog.C5ABeltZeroError;
            logRow.C5BBeltZeroError = dailyLog.C5BBeltZeroError;
            logRow.WaterLoss = dailyLog.WaterLoss;
            logRow.H2LeakRate = dailyLog.H2LeakRate;
            logRow.HoursOn = dailyLog.HoursOn;
            logRow.Boiler1HoursOn = dailyLog.Boiler1HoursOn;
            logRow.Boiler2HoursOn = dailyLog.Boiler2HoursOn;
            logRow.Comments = dailyLog.Comments;

            int rowsAffected = Adapter.Update(logTable);

            return rowsAffected == 1;
        }

        public bool DeleteDailyLog(int id)
        {
            DailyLogData.DailyLogDataTable logTable = Adapter.GetDailyLogsById(id);
            if (logTable.Count == 0)
                return false;

            DailyLogData.DailyLogRow logRow = logTable[0];

            int rowsAffected = Adapter.DeleteDailyLogById(id);

            return rowsAffected == 1;
        }
    }

[wiero]

i would suggest this improvements

- double has default value and you don't have set it's value to 0.0
- you can use AutoMapper to Map one object to another ( DailyLogRow to DailyLog and back )
- dont repeat yourself - make a method or extension to get IList<DailyLog> from DailyLogDataTable

This post has been edited by wiero: 10 February 2012 - 03:00 AM

[Sergio Tapia]

+1 - I'd use AutoMapper as well to avoid the repetition of left-right assignments. Other than that, nothing scream out to me as "bad".

[AdamSpeight2008]

erburrell,

Here my critic of your code, don't take it personally but you did ask us to critic your code.

---

Lack of comments.
So people can understand the purpose and intent of the code being written. It could be you in several years, updating or mostly likely fixing bugs.


So they can explain the differences in the number of repeat variables;-

  this.FeedTotalizer11_100 = 0.0;
  this.FeedTotalizer11_200 = 0.0;
  this.FeedTotalizer11_300 = 0.0;
  this.FeedTotalizer11_400 = 0.0;
  this.FeedTotalizer12_100 = 0.0;
  this.FeedTotalizer12_200 = 0.0;
  this.FeedTotalizer12_300 = 0.0;
  this.FeedTotalizer12_400 = 0.0;

This would could be taken as hint that you should use some form of collections.
The word Totalizer is used often, I would to strongly suggest (to me) that it should encapsulated into in own class named totalizer.

class Totalizer
{ .name : string /* if you want a named totalizer */
  .Totalizer : decimal
}

Now you can have any number of totalizers.

 Feed_Totalizers : List<Totalizer>

Also as I look through the code I see where words are shared across different variables. This suggests that related usage should be encapsulated into a separate classes. Both to convey semantic meaning, through the use of a type system (as bonus the compiler then helps you not to make errors, as the holes are different.) and to encapsulate information.

A few possible encapsulations.

Meter

Class Meter /* eg a gas meter not the distance Meter */
{
 .Reading : decimal
}

Sample

Class Sample 
{
 .No : String

Shouldn't be a string should really be type of SampleNo, as I suspect not that not every possible string value would a valid SampleNo. A SampleNo class could the check for valid entries, before accepting it.

 .Weight : Decimal /* NEEDS A Unit of Measure*/
}

I suggest wrapping decimal in to an encapsulated form, for semantic correctness

Spoiler

base UnitOfMeasure{}
base Weight : UnitOfMeasure
class Kgs : Weight
{
 .Value : Decimal
}

The units of measures would written in to a separate library class dll and imported, so its usage can be exploited by multiple projects.

Belt

Class Belt
{
 .ZeroError : decimal
 .Totalizer : Totalizer
}

Does C1BeltTotalizer have a ZeroError?
If so then it's missing,
if not then rewrite the belt class, and use inheritance to define a belt with a zero error present.

Class Belt
{
 .Totalizer : Totalizer
}
Class BeltWithError : Belt
{
 .ZeroError : decimal
}

Note both are a Type of belt.

This post has been edited by AdamSpeight2008: 10 February 2012 - 11:00 PM

[erburrell]

Adam,

WOW! Thanks for taking the time to do an excellent job of the critique. I really appreciate it, and will definitely be reviewing my code to modify it.

Thanks!

Ed.

[erburrell]

All,

As an update, I am posting my modified code to see if it is structured a bit better.

As always, any thoughts are always appreciated.

Please note that the DailyLog class is not finished. Only the base structure is there.

Thanks,

Ed

using System;

public class DailyLog
{
    public int Id { get; set; }
    public DateTime LogDate { get; set; }
    public IList<Meter> Meters;
    public IList<Totalizer> FeedTotalizers;
    public LigniteSample CoalSample;
    public Belt C1Belt;
    public IList<BeltWithError> Belts;
    public IList<RunHour> RunHours;
    public string Comments { get; set; }

    public DailyLog()
    {
        this.Id = 0;
        this.LogDate = DateTime.Now;
        this.Meters = new List<Meter>();
        this.FeedTotalizers = new List<Totalizer>();
        this.CoalSample = new LigniteSample();
        this.C1Belt = new Belt();
        this.Belts = new List<BeltWithError>();
        this.RunHours = new List<RunHour>();
        this.Comments = string.Empty;
    }
}

public class Belt
{
    public string Name;
    public Totalizer Total;
}

public class BeltWithError : Belt
{
    public double Error;
}

public class LigniteSample
{
    public string SampleNumber { get; set; }
    public EngineeringUnits.Forces.Ton SampleWeight { get; set; }
}

public class Meter
{
    public string Name;
    public double Reading;
}

public class RunHour
{
    public string Name;
    public EngineeringUnits.Times.Hour hours;
}

public class Totalizer
{
    public string Name { get; set; }
    public Ton Reading;
}


// Engineering Units Classes:

interface IUnit
{
    /// <summary>
    /// The "Official" Name of the Unit
    /// </summary>
    string Name
    {
        get;
        set;
    }

    /// <summary>
    /// The abbreviation of the Unit
    /// </summary>
    string Abbreviation
    {
        get;
        set;
    }
}

public abstract class Unit : IUnit
{
    public string Name
    {
        get { return Name; }
        set { Name = value; }
    }

    public string Abbreviation
    {
        get { return Abbreviation; }
        set { Abbreviation = value; }
    }

    public double Magnitude
    {
        get { return Magnitude; }
        set { Magnitude = value; }
    }

    public override string ToString()
    {
        return this.Magnitude + " " + this.Abbreviation;
    }
}

public class Force : EngineeringUnits.BaseClasses.Unit
{
    public EngineeringUnits.BaseClasses.Direction ActionDirection;

    public Force()
    {
        this.Magnitude = 0.0;
        this.ActionDirection = new BaseClasses.Direction();
    }

}

public class Ton : Force
{
    public Ton()
    {
        AssignLabels();
        this.Magnitude = 0.0;
        this.ActionDirection = new BaseClasses.Direction();
    }

    public Ton(double magnitude, double x, double y, double z)
    {
        AssignLabels();
        this.Magnitude = magnitude;
        this.ActionDirection = new BaseClasses.Direction(x, y, z);
    }

    private void AssignLabels()
    {
        this.Name = "Ton";
        this.Abbreviation = "ton";
    }
}

public class Direction
{
    public double x;
    public double y;
    public double z;

    public Direction()
    {
        this.x = 0.0;
        this.y = 0.0;
        this.z = 0.0;
    }

    public Direction(double x)
    {
        this.x = x;
        this.y = 0.0;
        this.z = 0.0;
    }

    public Direction(double x, double y)
    {
        this.x = x;
        this.y = y;
        this.z = 0.0;
    }


    public Direction(double x, double y, double z)
    {
        this.x = x;
        this.y = y;
        this.z = z;
    }

    public override string ToString()
    {
        return "(" + this.x + ", " + this.y + ", " + this.z + ")";
    }
}

public class Time : EngineeringUnits.BaseClasses.Unit
{
    public static Second ConvertMinToSec(Minute min)
    {
        return new Second(min.Magnitude * 60);
    }

    public static Second ConvertHourToSec(Hour hour)
    {
        return new Second(hour.Magnitude * 3600);
    }

    public static Minute ConvertSecToMin(Second sec)
    {
        return new Minute(sec.Magnitude / 60);
    }

    public static Minute ConvertHourToMin(Hour hour)
    {
        return new Minute(hour.Magnitude * 60);
    }

    public static Hour ConvertMinToHour(Minute min)
    {
        return new Hour(min.Magnitude / 60);
    }

    public static Hour ConvertSecToHour(Second sec)
    {
        return new Hour(sec.Magnitude / 3600);
    }

    public override string ToString()
    {
        return this.Magnitude.ToString() + " " + this.Abbreviation;
    }
}

public class Hour : Time
{
    public Hour()
    {
        AssignLabels();
        this.Magnitude = 0.0;
    }

    public Hour(double hour)
    {
        AssignLabels();
        this.Magnitude = hour;
    }

    private void AssignLabels()
    {
        this.Name = "Hour";
        this.Abbreviation = "h";
    }
}

[AdamSpeight2008]

To Me it's a lot better.
I'd separate out the Unit stuff into it own DLL so it can be utilized in future projects.

This post has been edited by AdamSpeight2008: 23 February 2012 - 01:14 PM

[eclipsed4utoo]

I would change the DailyLog class to this...

public class DailyLog
{
    public int Id { get; set; }
    public DateTime LogDate { get; set; }
    public IList<Meter> Meters = new List<Meter>();
    public IList<Totalizer> FeedTotalizers = new List<Totalizer>();
    public LigniteSample CoalSample = new LigniteSample();
    public Belt C1Belt = new Belt();
    public IList<BeltWithError> Belts = new List<BeltWithError>();
    public IList<RunHour> RunHours = new List<RunHour>();
    public string Comments { get; set; }

    public DailyLog()
    {
        this.LogDate = DateTime.Now;
    }
}

This gives the class properties their default values. The default value for ints are already 0, so no need to explicitly set it. The default value of strings are string.Empty, so no need to explicitly set it. The rest are just initializing lists and objects that can be done at declaration time.

The only one I left was the LogDate because that is not a default value.

Also, consider not making everything public. I don't know how you plan on using this, but only make objects public if you actually need to. Start by making everything private, and move out.

[AdamSpeight2008]

The classes should really be immutable, to change something create a new instance with changes.

1. Start by make the property Setters private to the class.

 public int Id { get; private set; }

2. Have private constructors
3. Create instance Static methods only, so validity checks can perform the object is constructed.

This post has been edited by AdamSpeight2008: 23 February 2012 - 02:11 PM

[janne_panne]

eclipsed4utoo: You dropped some properties out, making them just fields with declaring those default values.

enburrell:
I would also like to point out that since you are creating some sort of a log, can it be possible that 0.0 values are not wanted all the time?

For example in a situation where we are measing temperature and for some reason we can't get the temperature signal from the meter, we don't want to have 0.0 temperature because that would be wrong, rather we want to have a null value for values we weren't able to read.

The example might have been bad since maybe we want to log an error instead of any value, but we will rather have null than 0.0.

So you should consider if you need nullable values too. The syntax is: double? myValue = null;

But your code looks nice after the changes you made

[AdamSpeight2008]

janne_panne, on 23 February 2012 - 10:07 PM, said:

But your code looks nice after the changes you made

I have to agree with that.

[eclipsed4utoo]

janne_panne, on 23 February 2012 - 05:07 PM, said:

eclipsed4utoo: You dropped some properties out, making them just fields with declaring those default values.

I didn't change any of the 3 properties. I simply added the initialization to the public fields. There were only three properties in his new code.

[erburrell]

adam: I do have the unit stuff in a separate project so it can eventually become a dll for use in other projects. I do have one question though.

You state that I should make the class an immutable object and create new objects for changes. This class will be used to write/read data from a database, so how would you go about using it this way if it is immutable?

Thanks to all for the reviews and suggestions.

Ed.

[janne_panne]

eclipsed4utoo, on 24 February 2012 - 12:27 AM, said:

I didn't change any of the 3 properties. I simply added the initialization to the public fields. There were only three properties in his new code.

Oh yes, my bad. Maybe the fact that everything is public confused me.

So here is a another tip for enburrell: Make public fields into properties to follow OOP principles of encapsulation.
This way you can restrict the access to properties better than with just fields (you could make set method to other than public while get is still public).
For example, if you take a look at most .NET classes with any property which returns a collection, for example a ListBox which has Items collection, those properties is rarely settable, usually you can only call get method.

[AdamSpeight2008]

You make then Immutable to prevent accidental changes. If needs to change something then you have to explicitly create a new instance.

Example of an Immutable class

[System.ComponentModel.ImmutableObject(true)]
  public sealed class XY
  {
    public int X { get; private set; }
    public int Y { get; private set; }
    protected XY(int X,int Y) { this.X=X; this.Y=Y;    }
    public static XY operator  +  (XY a , XY B)/>{ return XY.CreateNew( a.X + b.X ,a.Y + b.Y); }
    public static XY CreateNew(int X , int Y)
    { //  Do Validation Checks on input data 
      return new XY(X,Y);
    }
  }

In use.

XY  a = XY.CreateNew( 1, 2 );
a.X = a.X + 2; // Error 
a.Y = a.Y + 2; // Error
// You're required to create a new instance
a = XY.CreateNew( a.X + 2, a.Y + 2 );

Quote

This class will be used to write/read data from a database, so how would you go about using it this way if it is immutable?

Database interaction is no concern of these classes. You create a class the does to manipulation for you. I can see that it needs at least to methods.

DataTable -> XY Read DataTable produce an XY

DataTable , XY -> DataTable Take a DataTable and a XY and write it contents to DataTable