locked
How to Handle created class instances (objects) properly? RRS feed

  • Question

  • User-1305530094 posted

    I am trying to have my tasks return same object response instead of having each one with a different return. I wrote the following class,

        public class ApiMethodResult : IDisposable {
            public ApiMethodResult () {
                Errors = new List<string> ();
                Warnings = new List<string> ();
                Value = null;
            }
            private dynamic Value { get; set; }
            private List<string> Errors { get; set; }
            private List<string> Warnings { get; set; }
            public void AddError (string errorMessage) {
                this.Errors.Add (errorMessage);
            }
            public void AddWarning (string warningMessage) {
                this.Warnings.Add (warningMessage);
            }
            public List<string> GetErrors () {
                return this.Errors;
            }
            public List<string> GetWarningList () {
                return this.Warnings;
            }
            public bool IsSuccess () {
                if (this.Errors.Count > 0)
                    return false;
                return true;
            }
            public bool IsWarning () {
                if (this.Warnings.Count > 0)
                    return true;
                return false;
            }
            public void AddValue (dynamic value) {
                this.Value = value;
            }
            public dynamic GetValue () {
                return this.Value;
            }
            private bool disposedValue = false;
            protected virtual void Dispose (bool disposing) {
                if (!disposedValue) {
                    if (disposing) {
                    }
                    disposedValue = true;
                }
            }
            void IDisposable.Dispose () {
                Dispose (true);
            }
        }

    I will explain more in this example,

            private async Task<ApiMethodResult> NotifyUser (ApplicationUser user, IdentityTracker tracker = null) {
                ApiMethodResult response = new ApiMethodResult ();
                try {
                    var code = await this.applicationUserRepository.GenerateEmailConfirmationTokenAsync (user);
                    var callbackUrl = Url.EmailConfirmationLink (user.Id, code, Request.Scheme);
    await templateService.SendNewAccountConfirmation (callbackUrl, user); await templateService.SendNewAccounSMStConfirmation (callbackUrl, user); return response; } catch (Exception ex) { ex.LogException ("3rd party error"); response.AddError (ex.Message); return response; } }
    await templateService.SendNewAccountConfirmation (callbackUrl, user);
    await templateService.SendNewAccounSMStConfirmation (callbackUrl, user);

    are two methods are using third party APIs, throwing an exception from any will stop the execution of my code, using "ApiMethodResult" will allow my code to continue and inspect the progress success as well. 

    I have two questions here,

    is it a bad practice implementing "ApiMethodResult" in all my code?

    if not, as shown above, I implemented "IDisposable" pattern, Do I need it or is it an overhead here?

    Thank you.

    Friday, June 22, 2018 5:49 AM

Answers

  • User-474980206 posted

    as you have nothing to release in dispose, you don't need to implement it. I question why Value is dynamic, rather then a generic.

    var response = new ApiMethodResult<MyValueType>();

     

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 22, 2018 3:46 PM
  • User753101303 posted

    Hi,

    Ultimately you need to work with a known response. In the sample you gave Value is left null which could be confusing but someone consuming your API. Also he'll need to know he should check both "Errors" (and even maybe "Warnings") each time. Finally it seems you swapped messages. You want to log ex.Message (or ex.ToString()) on your side and return a generic "3rd party error" to the API consumer.

    The "default" approach is usually to return an http status code (here you would get an http 500 "as usual"). If  not tried already I would keep something more HTTP and would improve as needed. If consumed by others it seems to me it makes things a bit harder to use right away with no obvious benefit.

    I wouild keep the web api http centric.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 22, 2018 4:56 PM

All replies

  • User-474980206 posted

    as you have nothing to release in dispose, you don't need to implement it. I question why Value is dynamic, rather then a generic.

    var response = new ApiMethodResult<MyValueType>();

     

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 22, 2018 3:46 PM
  • User753101303 posted

    Hi,

    Ultimately you need to work with a known response. In the sample you gave Value is left null which could be confusing but someone consuming your API. Also he'll need to know he should check both "Errors" (and even maybe "Warnings") each time. Finally it seems you swapped messages. You want to log ex.Message (or ex.ToString()) on your side and return a generic "3rd party error" to the API consumer.

    The "default" approach is usually to return an http status code (here you would get an http 500 "as usual"). If  not tried already I would keep something more HTTP and would improve as needed. If consumed by others it seems to me it makes things a bit harder to use right away with no obvious benefit.

    I wouild keep the web api http centric.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Friday, June 22, 2018 4:56 PM