views:

134

answers:

5

I'm not sure where to put some methods.

Let's say I want to send an email.

Which of the following options should I choose:

email = new Email("title", "adress", "body");
email.send();

or

email = new Email("title", "adress", "body");
Postman.send(email);

Because how can an email send itself? And isn't it better to have a central object that handles all emails because then he can regulate things like sending all emails at a specific time, sort mails, remove mails etc.

Also if I want to delete an user, how should I do:

user.delete();

or

administrator.delete(user);

Please share your thoughts about how to know where to put the methods.

+2  A: 

Use the second method to have a class manager handle the objects (emails or users). This follows the single-responsibility-principle.

Arseny
+8  A: 

I disagree with Arseny. An email can send itself, and that's exactly where the code should live. That's what methods are: actions that can be performed on the object.

However, note that your approaches are not mutually incompatible. An email's send action could easily just contain the code to add itself to the Postman's send queue, and if you do want to regulate the actions, that might be a good idea. But that's no reason not to have a send method for the email class.

Daniel Roseman
+1  A: 

In Ruby I'd do this:

email = Email.deliver(recipient, subject, message)

The correspoding class would look something like this:

class Email
  def self.deliver(recipient, subject, message)
    # Do stuff to send mail
  end
end

This is clean and easy to use.

On the delete issue: Delete the object you want to delete. So @user.delete would be best. If you want to register the administrator who deleted the user: @user.delete_by(@admin)

Ariejan
+3  A: 

All sensible methods that act on emails should be in the email class, for the convenience of users of your class. But email objects should not contain any fields except those related to the content of the email itself (single responsibility principle).

Therefore, I'd suggest this:

class Email
  def email(postman)
    postman.send(self)
  end
end

In statically typed languages, the type of the postman argument should definitely be an interface.

Michiel de Mare
+1  A: 

I agree with Daniel.

Following your first example, a lot of common widgets would also have a "collections" manager like you mentioned but they don't necessarily. A Tabs widget can show/hide one of its own tabs, without necessarily specifying a new Tab class for each individual one.

I believe functionality should be encapsulated. The example of deleting a user however, is a slightly different case. Having a delete method on the User class could do stuff like clear its own internal variables, settings, etc, but it won't delete the reference to itself. I find that delete methods are better suited for collection-based classes. I wouldn't per se put the delete method on a admin class but rather on a Users "collection" class.

function Users(){
    var users = [];
    this.add = function(user){
        // add user code
        users.push(new User(user));
    }
    this.remove = function(user){
        // remove user code and remove it from array
    }
}

I don't quite see how an object can fully add/remove itself so it makes sense to me to have that functionality at the collections level. Besides that though, I would say it should be encapsulated within the class it's meant for.

alex