0

I am quite new to C# and want to know the right way (not wasting any resources) to handle Generic Data.

I have a BindingList that is used to bind dataGridView.DataSource.

Below is the example code that updates Data values: The question is whether it be more efficient to place the "calculating codes" in the getter or the setter.

If there is an approach to this that would be considered a 'best practice' please let me know that too.

public class Data : INotifyPropertyChanged
{
    private float number;
    public string Code
    {
        get => Number; //  get => Number / 100     which one is more efficient?
        set { Number = value / 100; OnPropertyChanged(); }
    }
    public event PropertyChangedEventHandler PropertyChanged;
    private void OnPropertyChanged([CallerMemberName] string name = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(name));
    }
}

... where the Data class is used like so:

public partial class Form1 : Form
{
    private BindingList<Data> dataList = new BindingList<Data> { new Data { Code = "1"  } };

    protected override void OnHandleCreated(EventArgs e)
    {
        // When main form is ready to handle messages the binding happens.
        base.OnHandleCreated(e);
        myDataGridView.DataSource = dataList;
        myDataGridView.Columns["Code"].AutoSizeMode = DataGridViewAutoSizeColumnMode.Fill;
    }
    public Form1()
    {
        InitializeComponent();
    }
}
IVSoftware
  • 5,732
  • 2
  • 12
  • 23
hyukkyulee
  • 1,024
  • 1
  • 12
  • 17
  • Your code sample is interesting for pointing out that the underlying field type (in this case __float__) isn't always the same type as the property setting it (in this case __string__) and that we _do_ sometimes have to perform conversions in the {get; set;} – IVSoftware Jun 04 '20 at 19:12

1 Answers1

1

Q: "The question is whether to place "calculating codes" to the getter or not."

A: Short answer: Do the calculation once in the set instead of on every get.

Long answer:

I notice your 'Code' property is a string but the underlying value seems to be a float. That's ok, but I have a few suggestions for optimizing:

  1. Obviously, the input string value in the 'Code' property setter will have to be parsed at some point in order to perform the float calculation shown in your code. The float.Parse() method will throw a hard Exception as written here. Make sure you add some validation code to the input string and fail gracefully if it can't be parsed.
  2. To fire the PropertyChanged event, I suggest the form here: use the nameof keyword and make OnPropertyChanged protected virtual so that inherited classes can fire the event, too.

  3. It's just an opinion of course, but I would do the calculation once in the setter, not the getter. But then if it's stored as a float, I would do the ToString() in the getter.

  4. I don't see the harm in a short-running calculation in the setter. If it's some kind of long-running task to perform a calculation, consider using a method instead and possibly doing it asynchronously as a Task.


    public string Code
    {
        get => _number.ToString(); // 'Some' inefficiency here, but probably storing as float is preferred.
        set 
        {
            // I would offer that doing this ONCE in the setter
            // is more efficient than calculating on every get.
            float floatValue = float.Parse(value);  // Warning: Will throw exception 
                                                    // is value if unparsable input string
            _number = floatValue / 100;

            // Preferred form here is to use the 'nameof' keyword
            OnPropertyChanged(new PropertyChangedEventArgs(nameof(Code)));
        }
    }
    // I suggest some other notation than 'Number' because this
    // is a private field and when you say 'Number' the expectation is 
    // a public property. I see this notation often in Google source code
    // to indicate a 'private field' and have adopted it myself:
    private float _number;

    public event PropertyChangedEventHandler PropertyChanged;

    // This is customarily 'protected virtual'
    protected virtual void OnPropertyChanged(PropertyChangedEventArgs e)
    {
        PropertyChanged?.Invoke(this, e);
    }

Since a lot of this has to do with 'style points' there are other perspectives than my own. I do believe there's enough substance to justify posting this answer and hope it provides some insights for the thing you are asking.

IVSoftware
  • 5,732
  • 2
  • 12
  • 23
  • Amazing answer! This is exactly what I wanted especially on your point number 2,3,and 4 and For number 1 it was my mistake, it should be both float with same name. Yes I ended up trying all of three ways and like you said the code runs much faster once I put calculation in methods not in getters and setters Thank you IVSoftware :) – hyukkyulee Jun 04 '20 at 14:39
  • 1
    So glad I could assist! – IVSoftware Jun 04 '20 at 15:13
  • 1
    If you're digging into DataGridView, there are a few other answers I've written about the subject including [this one](https://stackoverflow.com/a/62153053/5438626) for starters. There are also more than a dozen Winforms examples on [IVSoftware @ GitHub](https://github.com/IVSoftware?tab=repositories) that are linked back to StackOverflow questions. You (and everyone) are certainly invited to look around and see if there's anything there you might think is worthwhile. – IVSoftware Jun 04 '20 at 15:33
  • 1
    @hyukkyulee I would say not so much a 'mistake' - the term I'd use is 'happy accident' because this scenario is _not_ unheard of. – IVSoftware Jun 04 '20 at 19:14
  • 1
    @IVSoftware I've had problems with using DataGridView in virtual mode. While updating my data (ex: adding/inserting a row), the DataGridView polls CellValueNeeded in the middle of the update, not enabling the background source to be updated (since both can't actually happen in parallel). I posted a [question](https://stackoverflow.com/questions/60658921/how-to-keep-virtualmode-datagridview-from-calling-cellvalueneeded-while-updating), but have gotten no feedback. In general, I have found the DataGridView control to be very buggy when used in virtual mode. – Adam Jun 04 '20 at 20:12
  • 1
    Evidence has it that DGV VirtualMode is non-buggy and delivers good performance if threading and message queues are well understood. You know I worked in automotive IC validation testing. [1 ppm error way too high](https://c44f5d406df450f4a66b-1b94a87d576253d9446df0a9ca62e142.ssl.cf2.rackcdn.com/2018/01/10_15-Packaging_ICs_to_Survive_the_Automotive_Environment.pdf) and detecting such errors requires even higher precision. (Sorry to repeat, I know that's in my SO profile...). DGV was our window to a bunch of SQLite databases that held millions of data points to cross-correlate. And it worked. – IVSoftware Jun 05 '20 at 22:15
  • 1
    @Adam I diagnosed your issue and have posted an [answer](https://stackoverflow.com/a/62236942/5438626) to your question. If you substitute my code for yours then the Exceptions you describe should no longer be thrown. – IVSoftware Jun 06 '20 at 22:10
  • @Adam and IVSoftware I am also working on virtualization right now and found your comments helpful thanks! – hyukkyulee Jun 07 '20 at 21:58
  • 1
    Hey @hyukkyulee I was pleased to see you (at least I think it's you) following our GutHub feed. So, in other words, you have both the [code](https://github.com/IVSoftware/data-grid-view-virtual-mode-gen-6.git) and the [video](http://ivsoftware.com/stack-overflow/data-grid-view-virtual-gen-6.mp4) for that project, yes? – IVSoftware Jun 07 '20 at 22:12
  • 1
    @IVSoftware Awesome :)))))!! yepp that's me always ready to learn great simple codes – hyukkyulee Jun 07 '20 at 23:33