Feedback on my design and how to use DCI Design Pattern?

Discussion in 'Ruby' started by josh, Dec 24, 2011.

  1. josh

    josh Guest

    I would like to get your feedback on the initial design of my app.
    The goal is to email a user when an event he cares about change it's time.
    i have User class, Event Class and an Email module.

    some questions:
    1. should Email be a class instead of a module?
    2. should Email be refactored into a method of the User class?
    3. I read a bit about DCI Design Pattern (http://www.clean-ruby.com/ and http://saturnflyer.com/blog/jim/201...what-your-system-is-vs-what-your-system-does/) and I am curious if there is a way to use it here.
    In DCI I believe I'll need a context module that will describe the main use cases of my app,
    and it each use case i'll add roles to some objects. in my case the user object will be extended with Email module (the Email will turn into a role, being able to send emails).

    I pasted my code below but it's nicer here: http://pastebin.com/XA6ez8FY

    # email a user when an event's time was changed.
    #
    # features
    # 1. user can add events (each event has a known url).
    # 2. every morning if any event's time was changed,
    # the user gets email with the changed events.
    # 3. on Monday morning, user gets email of all his events
    # regardless of changes in times.

    # holds a user and it's events
    class User
    attr_accessor :events, :name

    def initialize name
    @name = name
    @events = nil
    @updated = false
    end

    # ask each event to get it's current time and
    # mark the user as 'updated' so later on we
    # will email him/her
    def get_time
    @updated = @events.any?{|e| e.get_time; e.updated?}
    end

    def updated?
    @updated
    end
    end

    # holds info about a event
    class Event
    attr_accessor :name, :url, :base_time

    def initialize name, url, base_time
    @name = name
    @url = url
    @base_time = base_time
    @current_time = nil
    @updated = false
    end

    # scrape event from the website and mark it
    # as 'updated' if it's time was modified
    def get_time
    current_time = fetch_current_time
    @current_current_time = current_time if current_time
    if current_time != @base_time
    @updated = true
    end
    end

    def updated?
    @updated
    end

    private

    # get current event time from the website
    def fetch_current_time

    123124234324
    end
    end

    # send email to a user
    module Email
    module_function

    def send user
    # if it's Monday send email with all events
    # else send email with only the updated events
    puts "sending email to #{user.name}"
    end
    end

    user = User.new('josh')
    party = Event.new('party', 'http://events.com/events/123', 1234234)
    movie = Event.new('movie', 'http://events/events/564', 1243133)
    user.events = [party, movie]

    user.get_time

    if user.updated? or Date.now.monday?
    Email.send(user)
    end
    josh, Dec 24, 2011
    #1
    1. Advertising

  2. On 12/24/2011 01:16 PM, josh wrote:
    > I would like to get your feedback on the initial design of my app.
    > The goal is to email a user when an event he cares about change it's time.
    > i have User class, Event Class and an Email module.
    >
    > some questions:
    > 1. should Email be a class instead of a module?
    > 2. should Email be refactored into a method of the User class?


    With the current logic a module or class of its own seems overkill.
    However, if you need to do formatting etc. to send out an email a
    separate class might be appropriate. I'd probably construct an instance
    of Email for every User that is to receive an email, set all necessary
    properties (address, changed events) and then have a single method for
    sending (btw. #send might not be a good name because of Object#send).

    > 3. I read a bit about DCI Design Pattern (http://www.clean-ruby.com/ and http://saturnflyer.com/blog/jim/201...what-your-system-is-vs-what-your-system-does/) and I am curious if there is a way to use it here.
    > In DCI I believe I'll need a context module that will describe the main use cases of my app,
    > and it each use case i'll add roles to some objects. in my case the user object will be extended with Email module (the Email will turn into a role, being able to send emails).


    Frankly, I believe your application - which really seems to be a single
    use case only - is not complex enough to think about how to bring DCI
    into this. I have to admit, I just glanced over the text at
    http://www.artima.com/articles/dci_vision.html because I did not find it
    well structured, but the main point seems to be that functionality is
    not attached to individual classes but rather use cases and multiple
    classes. I believe this is what is called "multiple dispatch" elsewhere
    (Common Lisp has it IIRC).

    > I pasted my code below but it's nicer here: http://pastebin.com/XA6ez8FY


    There's one thing I'd change about your code: change @updated in Event
    from a boolean flag to the point in Time of last update (or throw it
    away since @base_time might have the proper semantics already) and that
    in User to point in time of last check. This has a few advantages

    - The email sending is a read only operation for Events (the flag does
    not need to be changed any more) and modification is restricted to
    methods which actually modify an Event.
    - If there are multiple clients interested in checking whether an Event
    has changed the code will break with a flag only (for example, you might
    want to send an administrator a single email per day with all changed
    events for monitoring or debugging purposes)
    - You can devise other action schemes (e.g. order Events in the email by
    modification time).

    Also, get rid of @current_time - this is what Time.now is for. This
    again makes querying a read only operation while currently you need to
    update @current_time.

    Kind regards

    robert
    Robert Klemme, Dec 25, 2011
    #2
    1. Advertising

  3. josh

    josh Guest

    Thanks for your insights Robert.

    Here is another question - should I pass the user object as argument
    of method that belong to email object or is it better to pass an email
    object into a method that belong to the user object?
    josh, Dec 26, 2011
    #3
  4. On 12/26/2011 08:23 PM, josh wrote:
    > Thanks for your insights Robert.
    >
    > Here is another question - should I pass the user object as argument
    > of method that belong to email object or is it better to pass an email
    > object into a method that belong to the user object?


    I'd probably do

    class User
    def send_updates(after = @updated)
    evts = @evetns.select {|e| e.updated >= after}

    unless evts.empty?
    em = Email.new
    em.user = self
    em.events = evts
    em.whatever_else = 123
    em.send_mail
    @updated = Time.now
    end
    end
    end

    You could as well also do this externally from User, e.g.

    class UpdateEmail
    attr_accessor :user, :events

    def self.send_updates(user)
    em = new
    em.user = user
    em.events = user.updated_events
    em.whatever_else = 123
    em.send_mail
    end
    end

    UpdateEmail.send_updates a_user

    Kind regards

    robert
    Robert Klemme, Dec 26, 2011
    #4
  5. josh

    josh Guest

    nice!

    I think i like the second option better.
    maybe I feel that the user should not be responsible for sending
    emails.
    I am curious if there is any advantage regarding testing it.
    I believe that in both cases I would stub the Email.send_email method.

    also, is it ok to send an email object to User.send_updates (the first
    option) instead of creating the object inside the method?
    not sure which one is considered better. it's just a few thoughts I
    always have when designing an app.
    josh, Dec 27, 2011
    #5
  6. On 27.12.2011 08:05, josh wrote:
    > nice!


    Thank you!

    > I think i like the second option better.
    > maybe I feel that the user should not be responsible for sending
    > emails.


    I tend to prefer the first option - probably because the instance of
    Email needs so much information that User does not necessarily have to
    exhibit publicly. I'd probably have to muse about it a bit more.

    Btw., I'd do slight changes:

    class User
    def send_updates(after = @updated)
    evts = @events.select {|e| e.updated >= after}
    now = Time.now # reduce risk of race conditions

    unless evts.empty?
    em = Email.new
    em.user = self # or set specific fields like mail_addr
    em.events = evts
    em.whatever_else = 123

    em.send_mail
    end

    # only set if sending was successful:
    @updated = now

    not evts.empty?
    end
    end

    > I am curious if there is any advantage regarding testing it.
    > I believe that in both cases I would stub the Email.send_email method.


    For testing you could override the method in Email that does delivery.
    Or you have an instance of a separate class for email delivery which
    then can be mocked.

    > also, is it ok to send an email object to User.send_updates (the first
    > option) instead of creating the object inside the method?


    Yes, you could reuse an Email instance. But that is only worthwhile
    IMHO if the cost of creating an Email instance is significant. If not
    I'd keep it an implementation detail of User so users of User do not
    need to know.

    > not sure which one is considered better. it's just a few thoughts I
    > always have when designing an app.


    Yep, and these change over time together with requirements and
    experience - so there is no all time best solution. But there aren't
    silver bullets either... :)

    Kind regards

    robert

    --
    remember.guy do |as, often| as.you_can - without end
    http://blog.rubybestpractices.com/
    Robert Klemme, Dec 27, 2011
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Pallav singh
    Replies:
    0
    Views:
    341
    Pallav singh
    Jan 22, 2012
  2. Pallav singh
    Replies:
    0
    Views:
    387
    Pallav singh
    Jan 22, 2012
  3. Pallav singh
    Replies:
    1
    Views:
    442
    Peter Remmers
    Jan 22, 2012
  4. Michel Demazure

    DCI and ruby

    Michel Demazure, Feb 13, 2011, in forum: Ruby
    Replies:
    3
    Views:
    110
    Andrzej K.
    Feb 13, 2011
  5. ChrisC
    Replies:
    4
    Views:
    158
    ChrisC
    Jun 25, 2010
Loading...

Share This Page