HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Helper function to return a string based on passed object and some defined rules

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
helperreturnpassedfunctionsomebasedandrulesobjectstring

Problem

In my application, a notification should be displayed when a chat message arrives.
There are some rules which decide what notification message is to be displayed.

I have created a helper method which takes a ChatMessage object and returns the message to be displayed in the notification.

How can I refactor or improve this piece of code?
Any feedback will be appreciated.

private String getNotificationDisplayString(ChatMessage chat) {
    String message;
    boolean isUpdateMessage = (chat.getTag().equals(TAG_UPDATE_MSG));
    if (isUpdateMessage) {
        // for update message
        UserRelationshipInfo relationshipInfo = UserRelationshipInfoManger.getInstance().getInfo(chat.getUserId());
        boolean isUserMyFriend = (relationshipInfo != null && relationshipInfo.getSource() == SOURCE_FRIEND);
        message = Resources.string(isUserMyFriend ? R.string.text_user_is_friend : R.string.text_user_is_stranger);
    } else {
        boolean isDirectMessage = (chat.getType() == DIRECT_MESSAGE);
        boolean isTextMessage = (chat.getTag().equals(TAG_TEXT_MSG));

        if (isDirectMessage) {
            // for direct message (special case)
            String messageName = Resources.string(getMessageType(chat.getTag()));
            // message = " sent you a direct "
            message = String.format(Resources.string(R.string.text_new_direct_message), chat.getUserInfo().getDisplayName(), messageName);
        } else if (isTextMessage) {
            // for text (non-direct)
            message = new TextInfo(chat).getText();
        } else {
            // for non-text (non-direct)
            // message = " sent you a message"
            message = String.format(Resources.string(R.string.text_new_message), chat.getUserInfo().getDisplayName());
        }
    }
    return message;
}

Solution

boolean isUpdateMessage = (chat.getTag().equals(TAG_UPDATE_MSG));


Consider using a chat.isUpdateMessage() method for that logic.

UserRelationshipInfo relationshipInfo = UserRelationshipInfoManger.getInstance().getInfo(chat.getUserId());


Singleton pattern detected! I would not recommend using the singleton pattern, I am quite certain that there's a better way to do this, as I don't know the entire context of your code I am not sure which method would be the best, but to get you started take a look at a previous answer of mine.

boolean isUserMyFriend = (relationshipInfo != null && relationshipInfo.getSource() == SOURCE_FRIEND);


Again, consider using as method such as relationshipInfo.isMyFriend()

message = Resources.string(isUserMyFriend ? R.string.text_user_is_friend : R.string.text_user_is_stranger);


Again, this seems like you have a Context stored in a static variable somewhere. I wouldn't recommend that. Tell, don't ask. If something needs your context, tell them about it. Don't have them ask some other object/place about it.

boolean isDirectMessage = (chat.getType() == DIRECT_MESSAGE);
boolean isTextMessage = (chat.getTag().equals(TAG_TEXT_MSG));


Two methods: chat.isDirectMessage() and chat.isTextMessage().

String messageName = Resources.string(getMessageType(chat.getTag()));


getMessageType should be a method in the ChatMessage class. chat.getMessageType(). And again, I'm not a fan of the Resources.string method as it tells me you got a static Context object somewhere.

message = String.format(Resources.string(R.string.text_new_message), chat.getUserInfo().getDisplayName());


When using Android resources, there's a special method for combining the getString with String.format. See this method

Code Snippets

boolean isUpdateMessage = (chat.getTag().equals(TAG_UPDATE_MSG));
UserRelationshipInfo relationshipInfo = UserRelationshipInfoManger.getInstance().getInfo(chat.getUserId());
boolean isUserMyFriend = (relationshipInfo != null && relationshipInfo.getSource() == SOURCE_FRIEND);
message = Resources.string(isUserMyFriend ? R.string.text_user_is_friend : R.string.text_user_is_stranger);
boolean isDirectMessage = (chat.getType() == DIRECT_MESSAGE);
boolean isTextMessage = (chat.getTag().equals(TAG_TEXT_MSG));

Context

StackExchange Code Review Q#51368, answer score: 4

Revisions (0)

No revisions yet.