7
\$\begingroup\$

I'm currently developing a method which performs an update of some data; much like the following simplified logic:

boolean updateRequired = (currentValue == null && newValue != null);
boolean deleteRequired = (currentValue != null && newValue == null);
if(updateRequired || deleteRequired) {
 // ...
}

Which probably is fine enough by itself. However, I can't loose the feeling that I'm somehow re-inventing the wheel. The above is just simplified, current/new-Value are strings if that matters.

Is there any way to make this more neat?

Malachi
29k11 gold badges86 silver badges188 bronze badges
asked Mar 14, 2014 at 17:21
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Is it c#, java or php? Your question is off-topic too, you need working and complete code to be that you want reviewed. You can have a language independent answer even if you specified which language you're using \$\endgroup\$ Commented Mar 14, 2014 at 17:24
  • \$\begingroup\$ If this is tagged with different language, then it may imply that it's example code. Such code is off-topic here. \$\endgroup\$ Commented Mar 14, 2014 at 17:40
  • \$\begingroup\$ This question appears to be off-topic because it is about example code, not the actual code you wrote. This question would be more suitable for 'Programmers'. \$\endgroup\$ Commented Mar 14, 2014 at 17:48
  • \$\begingroup\$ I've edited the question to be specified to Java. I thought to make the question as broad as possible (as this isn't really language specific). Stackoverflow-damaged ;-). \$\endgroup\$ Commented Mar 14, 2014 at 17:49

6 Answers 6

10
\$\begingroup\$

Depending on your use case, I recommend two options. Both of them are based on a separate function for checking things....

At its simplest, have a function:

private static final boolean needToDoSomething(String currentValue, String newValue) {
 return currentValue == null && newValue != null || currentValue != null && newValue == null;
}

Then you can just have:

if (needToDoSomething(currentValue, newValue)) {
 ....
}

If you need to do something more special than just the 'needs modification' test, for example, you need to distinguish inside the if block between these conditions, then I would recommend an Enum with a static method, for example:

public enum EditState {
 ADDED, DELETED, MODIFIED, UNCHANGED;
 public static getState(String currentValue, String newValue) {
 if (currentValue == newValue) {
 // this covers null == null too
 return UNCHANGED;
 }
 if (currentValue != null && newValue == null) {
 return DELETED;
 }
 if (newValue != null && currentValue == null) {
 return ADDED;
 }
 return currentValue.equals(newValue) ? UNCHANGED : MODIFIED;
 }
 public boolean isStateChanging() {
 return this != UNCHANGED;
 }
}

Then, with this Enum class, you can be more specific:

EditState state = EditState.getState(currentValue, newValue);
switch (state) {
 case UNCHANGED :
 .....
 case DELETED :
 .....
 ....
}
answered Mar 14, 2014 at 19:38
\$\endgroup\$
1
  • \$\begingroup\$ Nice, I really liked the enum solution; perhaps a bit overkill in my scenario, but nice. Thanks! \$\endgroup\$ Commented Mar 15, 2014 at 0:00
4
\$\begingroup\$

In my opinion, you haven't included enough code to make this a good Code Review question, but I'll try to answer anyway.

If currentValue == null and newValue != null, then it seems to me that createRequired would be a better name than updateRequired.

You appear to be checking to see if exactly one of currentValue or newValue is null.

if ((currentValue == null) != (newValue == null)) {
 // ...
}
answered Mar 14, 2014 at 22:51
\$\endgroup\$
3
\$\begingroup\$

Those few lines of code are very confusing

  • If the value was null and now not null, you are really creating or inserting, not updating. I would call it insertRequired

    boolean updateRequired = (currentValue == null && newValue != null);
    
  • Why would you have an if condition that triggers for both inserting and deleting, it would not make any sense. It should be

    if( insertRequired ){
     //doSomething
    }
    else if( deleteRequired ){
     //doSomething else
    }
    
answered Mar 14, 2014 at 18:45
\$\endgroup\$
3
  • \$\begingroup\$ Actually, since this is Java, that doesn't necessarily work. String a = "String"; String b = "String"; a != b == true; == and != compare references, which may be different for equivalent string content. You would use .equals() but you'd still have to do null checks to make sure at least one wasn't null. \$\endgroup\$ Commented Mar 14, 2014 at 18:49
  • 1
    \$\begingroup\$ Actually I just checked that and the example I gave does seem to give the same reference, but if you change it to String a = "String"; String b = new String("String");, then it has the expected effect. \$\endgroup\$ Commented Mar 14, 2014 at 18:56
  • \$\begingroup\$ @cbojar - Yep, Java uses the same instance for identical static strings. \$\endgroup\$ Commented Mar 15, 2014 at 7:42
2
\$\begingroup\$

I think I know what you are looking for. you only want to do stuff if

  • x = false and y = true
  • x = true and y = false

and not when

  • x = false and y = false
  • x = true and y = true

in that case I would think that what you want is an XOR or an Exclusive Or

boolean isNewValueAssigned = newvalue != null;
boolean isCurrentValueAssigned = currentValue != null;
if (isNewValueAssigned ^ isCurrentValueAssigned) {
 //Play more Galaga
 //do More Work
}

I think that this makes it more clear what you are trying to do.

Reference to this Comment and this Answer

answered Mar 14, 2014 at 18:54
\$\endgroup\$
1
  • 1
    \$\begingroup\$ The XOR operator in Java ^ is inended for use as a bitwise XOR, not a logical XOR of booleans. For a logical check, use != like if (updateRequired != deleteRequired) {....} \$\endgroup\$ Commented Mar 14, 2014 at 20:13
1
\$\begingroup\$

You could always condense it as follows:

boolean needToDoSomething = currentValue == null ? newValue != null : newValue == null;
if(needToDoSomething) {
 // ...
}

This is probably less clear than what you have, and gains you very little. Also, if you later branch based off of the individual values, you'd have to calculate them at that point anyway. In other words, probably best to leave it alone.

answered Mar 14, 2014 at 18:19
\$\endgroup\$
1
\$\begingroup\$

Just a thought I just had, what happens if both are false??


it looks like you have two separate Boolean variables that are meant to indicate two totally different things

  • Update
  • Delete

so really if this is the case you should leave the variables the way that they are and show us the rest of the code.

I imagine it looks something like this

boolean updateRequired = (currentValue == null && newValue != null);
boolean deleteRequired = (currentValue != null && newValue == null);
if (updateRequired) {
 object.Update();
} else if (deleteRequired) {
 object.Delete();
}

or maybe as two separate if statements altogether

if (updateRequired) {
 object.Update();
}
if (deleteRequired) {
 object.Delete();
}

I am betting you probably want to delete first and then insert the new data/object/whatever.


I looked at the question and it looks like you might want to delete first and then update/insert, if that is the case then you just need to flippy floppy the update and delete code.

answered Mar 14, 2014 at 18:28
\$\endgroup\$
2
  • 3
    \$\begingroup\$ You're assuming that he performs different operations whether it's an update or a delete. That isn't necessarily true (but it probably is :)). \$\endgroup\$ Commented Mar 14, 2014 at 18:35
  • \$\begingroup\$ there is a major structure issue here if OP is doing the same thing when one or the other are true. if both are false what happens? \$\endgroup\$ Commented Mar 14, 2014 at 18:38

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.