?
Solved

Any reason why this might be bad?

Posted on 2016-10-16
7
Medium Priority
?
52 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
  • 3
  • 3
7 Comments
 
LVL 41

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 2000 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
Windows Server 2016: All you need to know

Learn about Hyper-V features that increase functionality and usability of Microsoft Windows Server 2016. Also, throughout this eBook, you’ll find some basic PowerShell examples that will help you leverage the scripts in your environments!

 
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 2000 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

Veeam Disaster Recovery in Microsoft Azure

Veeam PN for Microsoft Azure is a FREE solution designed to simplify and automate the setup of a DR site in Microsoft Azure using lightweight software-defined networking. It reduces the complexity of VPN deployments and is designed for businesses of ALL sizes.

Question has a verified solution.

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

Many of us here at EE write code. Many of us write exceptional code; just as many of us write exception-prone code. As we all should know, exceptions are a mechanism for handling errors which are typically out of our control. From database errors, t…
For those of you who don't follow the news, or just happen to live under rocks, Microsoft Research released a beta SDK (http://www.microsoft.com/en-us/download/details.aspx?id=27876) for the Xbox 360 Kinect. If you don't know what a Kinect is (http:…
We’ve all felt that sense of false security before—locking down external access to a database or component and feeling like we’ve done all we need to do to secure company data. But that feeling is fleeting. Attacks these days can happen in many w…
Screencast - Getting to Know the Pipeline
Suggested Courses

807 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