Solved

Any reason why this might be bad?

Posted on 2016-10-16
7
47 Views
Last Modified: 2016-10-16
I'm running an async task. I have a callback class with a method called Update() which allows me to change the properties of the EventArgs class. Here's an abreviated version of the class:
    public class MyEventArgs : EventArgs
    {
        public string Message = string.Empty;
        public double Progress = 0;

        internal void Update(double progress)
        {
            this.Progress = progress;
            this.Message = string.Format("{0:f2}% complete", progress);
        }
    }

Open in new window

In the async method I use the following 2 lines:
                callbackProgress.Update(Math.Round((double)rowCount / (double)totalRows * 100, 0));
                callback(callbackProgress);

Open in new window

However, I could make the call a little cleaner by making the following change to the class:
    public class MyEventArgs : EventArgs
    {
        public string Message = string.Empty;
        public double Progress = 0;

        internal MyEventArgs Update(double progress)
        {
            this.Progress = progress;
            this.Message = string.Format("{0:f2}% complete", progress);
            return this;
        }
    }

Open in new window

I could then clean up the call like this:
                callback(callbackProgress.Update(Math.Round((double)rowCount / (double)totalRows * 100, 0));

Open in new window

So I'm wondering. Is there any reason I shouldn't do this? Are there downsides to this approach?
0
Comment
Question by:Russ Suter
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 3
  • 3
7 Comments
 
LVL 40

Expert Comment

by:Kyle Abrahams
ID: 41845859
Why are you returning anything at all?

You already have a reference to the object.  When you return itself you would essentially need to be saying:
callbackProgress = callbackProgress.Update(..);

have it be void:
        internal void Update(double progress)
        {
            this.Progress = progress;
            this.Message = string.Format("{0:f2}% complete", progress);       
        }

Open in new window


and the resulting call is still the same:
 callback(callbackProgress.Update(Math.Round((double)rowCount / (double)totalRows * 100, 0));
0
 
LVL 20

Author Comment

by:Russ Suter
ID: 41845863
That won't work. the callback variable (which is of type Action<MyEventArgs>) needs an object as its argument. if callbackProgress.Update(...) is the argument then effectively the code is callback(null) which won't work. That's why I'm returning the object reference.
0
 
LVL 30

Accepted Solution

by:
anarki_jimbel earned 500 total points
ID: 41845866
I can't really see much difference, you still pass the same object as an argument to a method.
However, I wouldn't do this as this is somehow different from a common pattern and does not really make your code cleaner. Might be confusing . I.e., fewer lines does not make code cleaner. For me :).
0
Salesforce Made Easy to Use

On-screen guidance at the moment of need enables you & your employees to focus on the core, you can now boost your adoption rates swiftly and simply with one easy tool.

 
LVL 30

Expert Comment

by:anarki_jimbel
ID: 41845869
callback(callbackProgress.Update(Math.Round((double)rowCount / (double)totalRows * 100, 0));

This won't work indeed.
0
 
LVL 20

Author Comment

by:Russ Suter
ID: 41845870
@anarki_jimbel

You might be right. It's possible I'm trying to be too clever here. I don't know that it really cleans anything up either. I was just musing. Thanks for your input.

I'm still unclear as to whether or not it actually has any effect on memory management. My gut tells me it doesn't since all I'm doing is passing a reference around.
0
 
LVL 30

Assisted Solution

by:anarki_jimbel
anarki_jimbel earned 500 total points
ID: 41845887
I believe you are correct. Basically, I believe, nothing changes - you still pass the same object.
What might be confusing for a consumer of your method: do we return the same object or Update method implementation replaces the object? And if it replaces - for what purpose?

Another thing - I am not usually keen to put everything into one line to make code more compact. Otherwise it often becomes less readable. And hard to debug.
0
 
LVL 20

Author Closing Comment

by:Russ Suter
ID: 41845950
Based on your advice I've decided to keep it simple. I do believe it's easier to read the more traditional way.
0

Featured Post

Space-Age Communications Transitions to DevOps

ViaSat, a global provider of satellite and wireless communications, securely connects businesses, governments, and organizations to the Internet. Learn how ViaSat’s Network Solutions Engineer, drove the transition from a traditional network support to a DevOps-centric model.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

A long time ago (May 2011), I have written an article showing you how to create a DLL using Visual Studio 2005 to be hosted in SQL Server 2005. That was valid at that time and it is still valid if you are still using these versions. You can still re…
Performance in games development is paramount: every microsecond counts to be able to do everything in less than 33ms (aiming at 16ms). C# foreach statement is one of the worst performance killers, and here I explain why.
In a recent question (https://www.experts-exchange.com/questions/29004105/Run-AutoHotkey-script-directly-from-Notepad.html) here at Experts Exchange, a member asked how to run an AutoHotkey script (.AHK) directly from Notepad++ (aka NPP). This video…

726 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question