0
\$\begingroup\$

I'm writing a code for my application: you can create a task, and you can choose the moment when you want to send this task (to some kind of user):

  • Send now
  • Send later
  • Repeat

Send options

You can select from above options. If you choose send later, you'll be prompted for a date:

Send later

And there are variety of options if you choose Repeat:

Repeat daily

(they're all on imgur if you're interested)

I use design-driven-development approach, and now I'm working on the view model for ASP.Net MVC application. Now it looks like this:

public class TaskViewModel
{
    // ... other properties (title, etc...)

    public SendOptions SendOption { get; set; } // when to send: now || later || repeat

    public RepeatDetails Repeat { get; set; }
    public LaterDetails Later { get; set; }

    // ==============================================
    // enumerations & classes
    // ==============================================

    public enum SendOptions { Now = 0, Later = 1, Repeat = 2}
    public enum RepeatTypes { Daily = 0, Weekly = 1, Monthly = 2, Yearly = 3 }
    public enum WeekDay {
        [EnumDescription("working day")]
        WorkingDay,
        [EnumDescription("weekend")]
        WeekEnd,
        [EnumDescription("sunday")]
        Sunday,
        [EnumDescription("monday")]
        Monday,
        [EnumDescription("tuesday")]
        Tuesday,
        [EnumDescription("wednesday")]
        Wednesday,
        [EnumDescription("thursday")]
        Thursday,
        [EnumDescription("friday")]
        Friday,
        [EnumDescription("saturday")]
        Saturday
    }
    public enum WeekDayNumber
    {
        [EnumDescription("1st")]
        First,
        [EnumDescription("2nd")]
        Second,
        [EnumDescription("3rd")]
        Third,
        [EnumDescription("4th")]
        Fourth,
        [EnumDescription("Last")]
        Last
    }
    public enum Month
    {
        [EnumDescription("January")]
        January,
        [EnumDescription("February")]
        February,
        [EnumDescription("March")]
        March,
        [EnumDescription("April")]
        April,
        [EnumDescription("May")]
        May,
        [EnumDescription("June")]
        June,
        [EnumDescription("July")]
        July,
        [EnumDescription("August")]
        August,
        [EnumDescription("September")]
        September,
        [EnumDescription("October")]
        October,
        [EnumDescription("November")]
        November,
        [EnumDescription("December")]
        December
    }

    // ==============================================
    // class for later screen details
    // ==============================================

    public class LaterDetails
    {
        public DateTimeOffset LaterUtc { get; set; }
    }

    // ==============================================
    // class for repeat screen details
    // ==============================================

    public class RepeatDetails
    {
        public RepeatTypes RepeatType { get; set; }
        public string StartAtTime { get; set; }
        public RepeatDailyDetails Daily { get; set; }
        public RepeatWeeklyDetails Weekly { get; set; }
        public RepeatMonthlyDetails Monthly { get; set; }
        public RepeatYearlyDetails Yearly { get; set; }
        public EndByDetails EndBy { get; set; }

        public class RepeatDailyDetails
        {
            public enum RepeatDailyTypes { EveryDay = 0, EveryWorkingDay = 1 }
            public RepeatDailyTypes RepeatDailyType { get; set; }
            public int EveryNthDay { get; set; }
        }

        public class RepeatWeeklyDetails
        {
            public int  EveryNthWeek    { get; set; }
            public bool IsSunday        { get; set; }
            public bool IsMonday        { get; set; }
            public bool IsTuesday       { get; set; }
            public bool IsWednesday     { get; set; }
            public bool IsThursday      { get; set; }
            public bool IsFriday        { get; set; }
            public bool IsSaturday      { get; set; }
        }

        public class RepeatMonthlyDetails
        {
            public enum RepeatMonthlyTypes { OnCertainDay = 0, At = 1 }
            public RepeatMonthlyTypes RepeatMonthlyType { get; set; }
            public int EveryNthMonth { get; set; }
            public string OnNthDay { get; set; }
            public WeekDayNumber WeekDayNumber { get; set; }
            public WeekDay WeekDay { get; set; }
        }

        public class RepeatYearlyDetails
        {
            public int EveryNthYear { get; set; }
            public Month Month { get; set; }
            public enum RepeatYearlyTypes { OnCertainDay = 0, At = 1 }
            public RepeatYearlyTypes RepeatYearlyType { get; set; }
            public string OnNthDay { get; set; }
            public WeekDayNumber WeekDayNumber { get; set; }
            public WeekDay WeekDay { get; set; }
        }

    }

    // ==============================================
    // class for end by details
    // ==============================================

    public class EndByDetails
    {
        public enum EndByTypes { NoEndDate = 0, EndAfter = 1, EndByDate = 2 }
        public EndByTypes EndByType { get; set; }
        public int Occurrences { get; set; }
    }

    public TaskViewModel()
    {
        Later = new LaterDetails();
        Repeat = new RepeatDetails()
        {
            Daily = new RepeatDetails.RepeatDailyDetails(),
            Weekly = new RepeatDetails.RepeatWeeklyDetails(),
            Monthly = new RepeatDetails.RepeatMonthlyDetails(),
            Yearly = new RepeatDetails.RepeatYearlyDetails(),
            EndBy = new EndByDetails()
        };
    }
}

EnumDescription attribute is used to create a dropdown list based on descriptions for enums:

public class EnumDescription : Attribute
{
    public string Text
    {
        get { return _text; }
    } private string _text;

    public EnumDescription(string text)
    {
        _text = text;
    }
}

I'm not satisfied with a code, and need a fresh look. What would you suggest to improve? Thanks in advance.

\$\endgroup\$
4
  • \$\begingroup\$ Does this pertain to all of these versions, or do you just need a single [asp.net-mvc] tag? \$\endgroup\$
    – Jamal
    Dec 11, 2013 at 13:07
  • 1
    \$\begingroup\$ @Jamal I think that multiple tags on a single question that differ only by version almost never make sense. On the other hand, it makes sense to have a general non-version tag, so I've gone ahead and created it. \$\endgroup\$
    – svick
    Dec 11, 2013 at 16:09
  • \$\begingroup\$ @svick: Good call. I was going to do it since SO has done the same, but I wasn't entirely sure. \$\endgroup\$
    – Jamal
    Dec 11, 2013 at 16:10
  • \$\begingroup\$ @Jamal, I just need a single tag \$\endgroup\$ Dec 11, 2013 at 16:32

3 Answers 3

3
\$\begingroup\$

Well one UI suggestion is where you can select to Repeat -> Daily -> Every working day.

  • This is very culture specific e.g., a Swede will work on the 4th of July while an American won't.

  • It also depends on the year as some holidays do not fall on the same date, and this takes us back to point 1

As for your code I just feel like I need to say that I'm allergic to comments like

// ==============================================
// class for repeat screen details
// ==============================================
\$\endgroup\$
1
  • 3
    \$\begingroup\$ I have the same feeling about this kind of commenting style. It would nicer to simply rename RepeatDetails to RepeatScreenDetails, and remove the whole comment imho. \$\endgroup\$
    – C4stor
    Dec 11, 2013 at 16:37
2
\$\begingroup\$

Some suggestions on reducing verbosity

Your EnumDescription is unnecessaryly verbose if you fallback to Enum.ToString as in this SO answer when description is not available, you could eliminate many description declarations e.g. months.

If you had another attribute that describes optional string transformations, you could eliminate the rest of the attributes.

public enum WeekDay {
    [EnumDescription("working day")]
    WorkingDay,
    [EnumDescription("weekend")]
    WeekEnd,
    [EnumDescription("sunday")]
    Sunday,
    [EnumDescription("monday")]
    Monday,
    [EnumDescription("tuesday")]
    Tuesday,
    [EnumDescription("wednesday")]
    Wednesday,
    [EnumDescription("thursday")]
    Thursday,
    [EnumDescription("friday")]
    Friday,
    [EnumDescription("saturday")]
    Saturday
}

becomes

[EnumDescriptionTransformations(
    new Transformations[]
    {
        Transformations.SplitCamelCase, 
        Transformations.ToLower
    })]
public enum WeekDay {WorkingDay,
    // I really want to name this enum WeekEnd 
    // even though it's grammatically incorrect
    [EnumDescription("weekend")]
    WeekEnd,
    // without attributes these can be put on a single line
    Sunday, Monday, Tuesday, Wednesday, Thursday, Friday, Saturday
}

Where Transformations is fixed number of common transformations.

public enum Transformations{ SplitCamelCase, ToLower, Capitalize, .....}

I think SplitCamelCase would be a sensible default and could be replaced by DoNotSplitCamelCase. A reminder: when you are lower-casing for aesthetic reasons, and not grammatical reasons as was this time, that would be left to CSS etc to handle.

You can also eliminate some common prefixes and improve readability. e.g. RepeatDetails.RepeatDailyDetails can be safely renamed to RepeatDetails.DailyDetails.

Also

    public class XXXDetails
    {
        public enum RepeatXXXTypes { ..... }
        public RepeatXXXTypes RepeatXXXType { get; set; }
        ......

can be safely converted to

    public class XXXDetails
    {
        public enum Types { ...... }
        public Types Type { get; set; }
        ......

-Details suffix is also unnecessary really, and serves only to prevent collision of property names and nested class names.

\$\endgroup\$
1
\$\begingroup\$

For the end by option, I would add an option "End after N occurences".

Also, by reading the code, I can't figure out what is the purpose of the variable "LaterUtc" in LaterDetails class.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.