Skip to content

Quiz #3 - Smell of Null Objects #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
arialdomartini merged 3 commits into from
Jul 25, 2014
Merged

Quiz #3 - Smell of Null Objects #14

arialdomartini merged 3 commits into from
Jul 25, 2014

Conversation

@arialdomartini
Copy link
Contributor

@arialdomartini arialdomartini commented Jul 25, 2014

The quiz

Here is a snippet of code I took (and adapted) from our production code.
Yes, it is real code, hence your Pull Requests could be truly accepted 🍺 🍺 🍺

public String Execute(final String file) {

        final String rewrittenUrl = fileHandler.getXmlFileFromFileName(file);
        final String executionId = fileHandler.getExecutionIdFromFileName(file);

        if ((executionId == "") || (rewrittenUrl == "")) {
            return "";
        }

        Foo knownFoo = fooRepository.getFooByXmlFileName(rewrittenUrl);

        if (knownFoo == null) {
            return "";
        }

        return knownFoo.DoThat(file);
    }

There are two conditional sections.

        if ((executionId == "") || (rewrittenUrl == "")) {
            return "";
        }

Now, Guard Clauses are not necessarily bad.
It would be very interesting to deliberately remove it, only in order to judge if the resulting code is better or worse

  • The second one is
  if (knownFoo == null) {
            return "";
        }

        return knownFoo.DoThat(file);

To me, it smells of missing Null Object, doesn't it?

arialdomartini added a commit that referenced this pull request Jul 25, 2014
@arialdomartini
Merge pull request #14 from arialdomartini/quiz3
aa0224d 
Quiz #3 - Smell of Null Objects
@arialdomartini arialdomartini merged commit aa0224d into code4nothing:master Jul 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

quiz

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@arialdomartini