?
Solved

Any reason why this might be bad?

Posted on 2016-10-16
7
Medium Priority
?
51 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 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
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
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

Concerto's Cloud Advisory Services

Want to avoid the missteps to gaining all the benefits of the cloud? Learn more about the different assessment options from our Cloud Advisory team.

Question has a verified solution.

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

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…
Exception Handling is in the core of any application that is able to dignify its name. In this article, I'll guide you through the process of writing a DRY (Don't Repeat Yourself) Exception Handling mechanism, using Aspect Oriented Programming.
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…
Are you ready to place your question in front of subject-matter experts for more timely responses? With the release of Priority Question, Premium Members, Team Accounts and Qualified Experts can now identify the emergent level of their issue, signal…

649 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