-1
\$\begingroup\$

Is this code good for checking if the variable texto is empty or not? How would it be done with try and catch?

public class RedProfesionalException extends Exception {

    public RedProfesionalException(String message) {
       super(message);
    }
}

public class Comentari extends Object {
    private String autor, texto;

    public Comentari(String autor, String texto) throws RedProfesionalException{
        this.autor = autor;
        this.texto = texto;
        if (texto==null || texto.equals(""))
            throw new RedProfesionalException("You have written nothing");

    }

}
\$\endgroup\$
  • 2
    \$\begingroup\$ There is nothing wrong with your current code... \$\endgroup\$ – fge May 26 '13 at 20:49
  • 2
    \$\begingroup\$ Please consider converting your names/variables to English before posting it here. In this case it does not matter much, but if the examples get more complicated it's a necessity. \$\endgroup\$ – Bobby May 27 '13 at 7:33
  • \$\begingroup\$ Does the Comentari class has an ID field not shown here? Does it have setters/getters (especially setters) for autor and texto also not shown here? \$\endgroup\$ – abuzittin gillifirca May 27 '13 at 10:41
1
\$\begingroup\$

I disagree. This code should be improved.

try
{
      String autor = "SomeAuthor";
      String texto = null;

      Comentari newComentari = new Comentari(autor, texto);
}
catch {RedProfesionalException ex)
{
  // React to catched exception here...
}

That is not how object orientation works. It is a bad habit to throw exceptions in a constructor:

public class Comentari extends Object {
    private String autor, texto;

    public Comentari(String autor, String texto) throws RedProfesionalException{
        this.autor = autor;
        this.texto = texto;
        if (texto==null || texto.equals(""))
            throw new RedProfesionalException("You have written nothing");

    }

}

One reason to choose OOP is the advantage of creating an object in a predefined state. What you wanted is: you wanted to create a valid Comentari object. What you did is: in case the state of your object would be invalid, your object would be created and just break in to pieces because of your exception. It would be much cleaner to wrap this into a factory function like:

public Comentari createComentariWithAuthorAndText(String autor, String texto){
    if (texto==null || texto.equals(""))
        throw new RedProfesionalException("You have written nothing");
    return new Comentari(author, texto);
}

So there is nerver an inconsistent object created and no objects were harmed ;) Another solution would be to allow the creation of an empty commentary, but check later, if comentari.getTexto() is empty and handle that fact later.

So the try/catch looks like:

try
    {
          String autor = "SomeAuthor";
          String texto = null;

          Comentari newComentari = createComentariWithAuthorAndText(autor, texto);
    }
    catch {RedProfesionalException ex)
    {
      // React to catched exception here...
    }
\$\endgroup\$
  • \$\begingroup\$ Oh I agree completely, good point. I didn't take it any further because the question seemed to be more about a misconception or misunderstanding of what to do with the actual try/catch once an exception is thrown. But +1 for taking the design further. Definitely the best answer. \$\endgroup\$ – CodingBeagle May 27 '13 at 9:06
  • \$\begingroup\$ >*So there is nerver an inconsistent object created ...* If you have a public constructor, as is the case here, all bets are off. \$\endgroup\$ – abuzittin gillifirca May 27 '13 at 10:24
  • \$\begingroup\$ It should be more a metaphor: the object is killed in statu nascendi so to say. Nevertheless I think, throwing exceptions in constructors is not a nice behaviour. \$\endgroup\$ – Thomas Junk May 27 '13 at 11:32
1
\$\begingroup\$

The current way of throwing your exception is absolutely fine from what I can see, I'm not quite sure what your question is supposed to be as since you want to throw an exception, you do not actually want to use try/catch. That part is left for the client code.

Point is, the code you have right now is just fine :)

EDIT

Perhaps you mean that you have to write code which makes use of the class to test if an exception has been thrown? In that case it would look something like:

try
{
      String autor = "SomeAuthor";
      String texto = null;

      Comentari newComentari = new Comentari(autor, texto);
}
catch (RedProfesionalException ex)
{
      // React to caught exception here...
}

Perhaps this is what you want?

\$\endgroup\$
  • \$\begingroup\$ I am asked to do it with try and catch, do you know how? \$\endgroup\$ – user25496 May 26 '13 at 20:58
  • \$\begingroup\$ I have edited the answer above with additional information. Is this what you are trying to accomplish? \$\endgroup\$ – CodingBeagle May 26 '13 at 21:08
  • \$\begingroup\$ The javadoc says me that RedProfesionalExceptions throws when the text is empty, with the following text: "you must write something!" \$\endgroup\$ – user25496 May 26 '13 at 21:23
  • \$\begingroup\$ Which is correct :-) I'm sorry but I'm having a hard time understanding exactly what you want to do besides what has already been shown. Perhaps another person knows what it is you are trying to achieve :-) \$\endgroup\$ – CodingBeagle May 27 '13 at 7:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.