search

notaphplover / catan-heroes

A variant of the classic game
MIT License
0 stars 1 forks source link

Add new exception to check is the owner is null. #48

Open abvg9 opened 4 years ago

abvg9 commented 4 years ago

In line 177 at the file src/com/ucm/dasi/catan/board/CatanBoard.java it's recommended to implement a runtime expcetion to handle this situation.

notaphplover commented 4 years ago

@abvg9 Try to reference code with GitHub references in base of a certain commit. For example, the line you are referencing is this one

notaphplover commented 4 years ago

@abvg9 Mhmm I think maybe the code should allow that.

In the actual scenario there are no chances to get an structure with no owner since TerrainType.NONE is banned, but what if we implement a terrain with no owner, such an obstacle? we shouldn't code in base of the current scenario, we should code keeping in mind the desired behavior we want to implement. And the desired behavior is a calculus of the structures with owner.

What do you think about this?

abvg9 commented 4 years ago

So why did we have an if that checked that? If it was an unnecessary case then of course it would not be necessary to develop the exception, however, if the revision did make sense, it is recommended to treat it as an exception. Perhaps with a RuntimeException since it allows to avoid the treatment with a try catch.

notaphplover commented 4 years ago

So why did we have an if that checked that? If it was an unnecessary case then of course it would not be necessary to develop the exception, however, if the revision did make sense, it is recommended to treat it as an exception. Perhaps with a RuntimeException since it allows to avoid the treatment with a try catch.

As I said previously:

Those are the reason to place the if block. It's not an unnecessary case, but an exception does not make sense because it's a valid case.

abvg9 commented 4 years ago

So ... how do you know what happened if the function exits for that return? Is it a function that if you call it, it has no effect?

notaphplover commented 4 years ago

how do you know what happened if the function exits for that return?

Who is "you" in that context? Probably that "you" has nothing to take care about that. Having said that, that "you" probably could realize that no production increment was performed.

Is it a function that if you call it, it has no effect?

Sometimes yes as you can figure looking at the code

abvg9 commented 4 years ago

That you refer to those who review the code at runtime to analyze what has happened. In the event that a behavior occurs and one does not know how to differentiate between what has occurred, it generates uncertainty and causes the traceability of the code to be lost. In general, it is malpractice.

notaphplover commented 4 years ago

That you refer to those who review the code at runtime to analyze what has happened. In the event that a behavior occurs and one does not know how to differentiate between what has occurred, it generates uncertainty and causes the traceability of the code to be lost. In general, it is malpractice.

It would be a bad practice if that was an unexpected behavior, if you say that components should not fail silently, I agree with you. But this is not a sillent error, its the desired behavior.

it generates uncertainty and causes the traceability of the code to be lost. In general, it is malpractice.

As said previously:

Having said that, that "you" probably could realize that no production increment was performed.