0

I am working on an application where I have implemented a functionality. The flow is like we are getting settings that can be 1 HOURS, 2 HOURS, 1 WEEK, 2 WEEKS or 1 MONTH. For Example, If I get Settings from Database like 1 HOUR then it will calculate the difference between two dates and check for 1 HOUR with the difference I am getting from the calculation. I have done it by using multiple IF checks but I want to optimize the code. Here is my implementation.

public enum TaskSchedulingEnum : int
    {
        [Description("1 HOUR")]
        OneHour = 1,
        [Description("2 HOURS")]
        TwoHours = 2,
        [Description("8 HOURS")]
        EightHours = 8,
        [Description("1 DAY")]
        OneDay = 1,
        [Description("3 DAYS")]
        ThreeDays = 3,
        [Description("1 WEEK")]
        OneWeek = 1,
        [Description("2 WEEKS")]
        TwoWeeks = 2,
        [Description("1 MONTH")]
        OneMonth = 1,
        [Description("ALWAYS")]
        Always = 14
    }

private async Task<bool> GetUserSettings(string companyId, DateTime? TaskStartDateTime)
        {


            // get company settings ... 
            var settings = await projulSettingsService.GetCompanyNotificationSettings(companyId, ProjulSettingsTypeEnum.Notifications);
            var scheduleSettings = string.Empty;
            if (settings.Where(x => x.Name == ProjulPortalSettings.GeneralSettingsNotificationIsDisabled).Select(x => x.Value).FirstOrDefault() == "false")
            {
                scheduleSettings = JsonConvert.DeserializeObject<string>(settings.Where(x => x.Name == ProjulPortalSettings.NotificationTaskAssignedImmediateWindow).Select(x => x.Value).FirstOrDefault());
                if (!string.IsNullOrEmpty(scheduleSettings))
                {
                    var _timespan = (DateTime.UtcNow - TaskStartDateTime).Value;
                    var difference = _timespan.TotalHours; // in hours..
                    TimeSpan days = TimeSpan.FromHours(difference); // days..
                    var weeks = (days.TotalDays % 365) / 7; // weeks 
                    var months = (days.TotalDays % 365) / 30; // months

                    var list = ApplicationExtensions.GetEnumList<TaskSchedulingEnum>().ToList();
                    var _val = list.Where(x => x.Text == scheduleSettings).FirstOrDefault();

                    if (scheduleSettings == TaskSchedulingEnum.OneHour.GetEnumDescrip()
                        || scheduleSettings == TaskSchedulingEnum.TwoHours.GetEnumDescrip()
                        || scheduleSettings == TaskSchedulingEnum.EightHours.GetEnumDescrip())
                    {
                        if (difference == Convert.ToDouble(_val.Value))
                            return true;
                    }
                    else if (scheduleSettings == TaskSchedulingEnum.OneDay.GetEnumDescrip()
                         || scheduleSettings == TaskSchedulingEnum.ThreeDays.GetEnumDescrip())
                    {
                        if (days.TotalDays == Convert.ToDouble(_val.Value))
                            return true;
                    }
                    else if (scheduleSettings == TaskSchedulingEnum.OneWeek.GetEnumDescrip()
                        || scheduleSettings == TaskSchedulingEnum.TwoWeeks.GetEnumDescrip())
                    {
                        if (weeks == Convert.ToDouble(_val.Value))
                            return true;
                    }
                    else if (scheduleSettings == TaskSchedulingEnum.OneMonth.GetEnumDescrip())
                    {
                        if (months == Convert.ToDouble(_val.Value))
                            return true;
                    }
                    else if (scheduleSettings == TaskSchedulingEnum.Always.GetEnumDescrip())
                    {
                        return true;
                    }
                    return false;

                }
            }
            return false;
        }

Is there any way to write optimized and less code to achieve the functionality? I am getting the same settings from the database as mentioned in the Enum description. That can be a single value every time. Always check will return true always.

Muhammad Kamran
  • 105
  • 1
  • 11
  • 2
    You are mis-using enums if you have duplicate values. You might want a different data structure, like a Dictionary perhaps. That aside, SO is not for optimization questions on working code. Maybe use [codereview.stackexchange](https://codereview.stackexchange.com/) – Crowcoder Jul 20 '22 at 12:29
  • mis-using is a rather Understatement IHMO that’s just plain WRONG. enumerated should only have the same int value if they are the identical just a different name. – Rand Random Jul 20 '22 at 12:31
  • https://stackoverflow.com/questions/8043027/non-unique-enum-values – Rand Random Jul 20 '22 at 12:32
  • I don't to split *1 HOURS* and get 1 from the value that is the reason I have define duplicate values for the enum. But I am filtering the enum by using the description – Muhammad Kamran Jul 20 '22 at 12:33
  • @Crowcoder Can you please elaborate or provide an example which helps me improve my code. – Muhammad Kamran Jul 20 '22 at 12:34
  • @Crowcoder "SO is not for optimization questions on working code"? There's an optimization tag and plenty of questions on the topic. You have a reference for that? – Asik Jul 20 '22 at 14:21
  • @Asik maybe "not for" isn't true, but I think generally [such questions](https://meta.stackoverflow.com/questions/261841/can-i-post-questions-about-optimizing-code-on-stack-overflow) are not well received and may be more appropriate for code review. – Crowcoder Jul 20 '22 at 14:27

1 Answers1

1

I would not recommend using enum values in this way. Let each enum value represent an arbitrary tag, that you separately convert to a duration with a extension method. For example:

        public static TimeSpan ToDuration(this TaskSchedulingEnum self)
        {
            return self switch
            {
                TaskSchedulingEnum.OneHour => TimeSpan.FromHours(1),
                TaskSchedulingEnum.TwoHours => TimeSpan.FromHours(2),
                TaskSchedulingEnum.OneDay => TimeSpan.FromDays(1),
                TaskSchedulingEnum.Always => Timeout.InfiniteTimeSpan,
            };
        }

Once you have a timespan to describe the duration it should be trivial to add this to any start time to get the end time or other similar operations.

Note that you might need special handling for the Always-value, since arithmetic operations with infinities will not work correctly. You might want to describe it with TimeSpan.MaxValue or use a TimeSpan? type instead, depending on how it is used.

JonasH
  • 28,608
  • 2
  • 10
  • 23