Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is constantly making new objects normal?

ANSWER: Even though turning the classes into variables instead and then calling them would work, in this case it is likely a sign of bad design. A better design will have the side-effect that constantly making new instances of classes isn't required to begin with.

============

Lately I've been (attempting to) apply the SRP to my code, splitting up all responsibilities to separate classes. It has worked very well, maintainability and reusability of code went up tons.. yet I find myself constantly doing things like new Table.Type("messages").Get(id);.

Most of my classes contain only one method. Still.. it feels awkward. Guts say this is the point where I might as well turn them all into static classes.

So I figured I'd turn to my more experienced seniors, is frequently writing 'new Class().Method()' commonly done? Or is there a better way to handle it?

Example code:

public void AdminCommands(Channel channel, IrcUser user, string message)
{
    var command = message.Split(' ')[0];
    switch (command)
    {
        case "@info":
            GetInfo(channel, message);
            break;
        //----a couple of more commands
    }
}

private void GetInfo(Channel channel, string message)
{
    Match match = Regex.Match(message, "@info (.+)");
    if (match.Success)
    {
        string search = match.Groups[1].Value;
        //Get stored data on the word or sentence, and send the result to chat.
        new CommandInfo().Execute(search);  //<-------------------- over here.
        return;
    }
    Chat.SendAdminMessage("Message not found.");
}
private void EditMessage(Channel channel, string message)
{
    Match match = Regex.Match(message, "@edit (.+?) (.+?) (.+?)=(.+)");
    if (match.Success)
    {
        string type = match.Groups[1].Value;
        string id = match.Groups[2].Value;
        string toReplace = match.Groups[3].Value;
        string replaceWith = match.Groups[4].Value;

        //Gets message of 'type' by 'id', and store it back after editing.
        new CommandEdit().Execute(type, id, toReplace, replaceWith); //<-here.
    }
}
like image 894
Taelia Avatar asked Feb 13 '23 16:02

Taelia


2 Answers

you can of course store CommandEdit and CommandInfo as member variables and then call Execute

private CommandInfo mInfo = new CommandInfo();

private void GetInfo(Channel channel, string message)
{
    Match match = Regex.Match(message, "@info (.+)");
    if (match.Success)
    {
        string search = match.Groups[1].Value;
        //Get stored data on the word or sentence, and send the result to chat.
        mInfo.Execute(search);  //<-------------------- over here again.
        return;
    }
    Chat.SendAdminMessage("Message not found.");
}
like image 179
Serve Laurijssen Avatar answered Feb 23 '23 22:02

Serve Laurijssen


It's difficult to tell from your example code, but you seem to be creating procedural code with a pretend overcoat of object orientation.

I can't tell anything about your code from your object model. For instance, what exactly does this line do?

new CommandInfo().Execute(search);

What is a CommandInfo object? What does it represent? What on earth does an Execute() function do? I have no idea.

The whole point of objects is that they encapsulate some kind of internal state. If you are constantly newing things up then they obviously don't have any state.

A lot of your code seems to be operating on messages, so why not put methods that operate on messages on the message class?

Message.GetInfo();
Message.Edit();

etc. It's difficult to suggest a model when I can't work out what the code it doing, but can you see how much more obvious this makes the code?.

like image 33
GazTheDestroyer Avatar answered Feb 23 '23 21:02

GazTheDestroyer