Solved

Any reason why this might be bad?

Posted on 2016-10-16
7
49 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
Monthly Recap

May was a big month for new releases from Linux Academy! Take a look at what our team built recently in our blog. You can access the newest releases from our blog.

 
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

The Orion Papers

Are you interested in becoming an AWS Certified Solutions Architect?

Discover a new interactive way of training for the exam.

Question has a verified solution.

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

We all know that functional code is the leg that any good program stands on when it comes right down to it, however, if your program lacks a good user interface your product may not have the appeal needed to keep your customers happy. This issue can…
More often than not, we developers are confronted with a need: a need to make some kind of magic happen via code. Whether it is for a client, for the boss, or for our own personal projects, the need must be satisfied. Most of the time, the Framework…
If you’ve ever visited a web page and noticed a cool font that you really liked the look of, but couldn’t figure out which font it was so that you could use it for your own work, then this video is for you! In this Micro Tutorial, you'll learn yo…
Do you want to know how to make a graph with Microsoft Access? First, create a query with the data for the chart. Then make a blank form and add a chart control. This video also shows how to change what data is displayed on the graph as well as form…

688 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