Is this safe inside constructor or should it move elsewhere

Question

I'm not sure if my code is safe upon an error inside constructor what will happen? Should I change the way I wrote the code or is just fine?

What happen to a new obj that constructor throw null exception or fail to executed?

private Response _response = Response.NONE;
private long _time = System.currentTimeMillis();

public Request(final Site site)
{
    final Holder holder = Holder.getInstance().getHolder(site);

    if (holder == null)
    {
        throw new NullPointerException("Failed to find holder");
    }

    HttpURLConnection connection = null;

    try
    {
        final URL url = new URL(site.toString());

        connection = (HttpURLConnection) url.openConnection();
        connection.setRequestProperty("User-Agent", "");
        connection.connect();

        final String streamResponse = new BufferedReader(new InputStreamReader(connection.getInputStream())).lines().collect(Collectors.joining("\n"));

        switch (site)
        {
            case X:
                _response = Response.SUCCESS;
                _time = <some code here>
                break;
        }
    }
    catch (final IOException e)
    {

    }
    finally
    {
        if (connection != null)
        {
            connection.disconnect();
        }
    }
}

public Response getResponse()
{
    return _response;
}

public long getTime()
{
    return _time;
}

[Highly relevant](https://stackoverflow.com/questions/7048515/is-doing-a-lot-in-constructors-bad): "Is doing a lot in constructors bad?" — Andy Turner, May 18 '20 at 15:26
@AndyTurner is right. It has been already answered in that question. You also have a syntax error in your code at `throw new NullPointerException("Failed to find holder);` missing `"` — mxsxs2, May 18 '20 at 15:55

score 0 · Answer 1 · answered May 18 '20 at 15:20

It depends on which constructor you are talking about. Currently, if there is an error, it would only be caught if it occurred in the try/catch statement. Even then, you haven't implemented the catch statement to handle the error, so if there is an error in the try/catch, nothing would happen in its current state. You might add a console log to print out the error; but, it depends on what your overall approach is to handling errors.

If you want something a little more flexible in terms of catching errors, you can have you constructor throw and error like so:

public Request(final Site site) throws NullPointerException, IOException, ..//any other errors

If there was any other type of error that you were getting in the method, you would add it to the method signature. If you do this, you would still have to handle the error, but instead of handling it in the method here, you would handle it where to call the Request constructor.

So ultimately, it depends on how you want to handle your error and where you want to deal with it.

So let's ignore the error and i know i have some syntax errors. I edited manually the code to make it shorter for stackoverflow. Is this fine? To have such a code inside a constructor that read from from website? — WhatRemainsOfEdithFinch, May 18 '20 at 16:06

Is this safe inside constructor or should it move elsewhere

1 Answers1