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
Solved

Any reason why this might be bad?

Posted on 2016-10-16
7
46 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 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 29

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
Master Your Team's Linux and Cloud Stack!

The average business loses $13.5M per year to ineffective training (per 1,000 employees). Keep ahead of the competition and combine in-person quality with online cost and flexibility by training with Linux Academy.

 
LVL 29

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 29

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

Free Tool: Subnet Calculator

The subnet calculator helps you design networks by taking an IP address and network mask and returning information such as network, broadcast address, and host range.

One of a set of tools we're offering as a way of saying thank you for being a part of the community.

Question has a verified solution.

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

Suggested Solutions

Title # Comments Views Activity
Expression Evaluater 3 37
VB.NET 2008 Winforms Signing 13 31
jquery tab header text 1 23
linq, c# 8 22
In my previous two articles we discussed Binary Serialization (http://www.experts-exchange.com/A_4362.html) and XML Serialization (http://www.experts-exchange.com/A_4425.html). In this article we will try to know more about SOAP (Simple Object Acces…
Wouldn’t it be nice if you could test whether an element is contained in an array by using a Contains method just like the one available on List objects? Wouldn’t it be good if you could write code like this? (CODE) In .NET 3.5, this is possible…
This video shows how to use Hyena, from SystemTools Software, to bulk import 100 user accounts from an external text file. View in 1080p for best video quality.

839 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