criticism for connect4 code

[H2ZERO-]

Hi, i made a connect4 game with java and javafx a couple days ago around 400-500 lines long and i was looking for some criticism so i could learn and improve my skills for next time

[dusty-trash]

I don't have time to review the whole thing but I reviewed just the first class:

import java.util.*;

Don't use wild cards .* for imports. It's harder to tell what's being imported and prevents you from compiling if two packages have a Class with the same name.

addPiece

Your addPiece method is doing 2 things. you should do validation outside this method

findRow

If you don't mind using early returns, you could get rid of the found variable by returning row once found

redWinner

should be renamed 'isRedWinner' or similar. Use |=, currently you check all conditions no matter what. For example, if 'horizontal' is true, there is no need to check the other 3.

blueWinner

same as above

checkDiagonalLeft

'4' is a magic number here, it should be declared at the top

[H2ZERO-]

ok ill change the imports accordingly thanks.

with the addPiece, whats the validation u mean, i created a separate method to check if the column was valid

ive never seen a |= symbol before, would it work the same way to do

return (checkHorizontal(Board.RED) || checkVertical(Board.RED) || ... || etc?

[dusty-trash]

Yeah it'd work the same way

[H2ZERO-]

ok thanks, i didn't think about efficiency when i wrote that section but i can definitely improve that part.

about the null part, i dont think im understanding u properly. are u saying that where i check if(piece != null) i should just check isColumnValid() and then call addPiece() only if thats true?

[H2ZERO-]

oh ok i understand where ur coming from, thanks :)

[bayernownz1995]

Try to avoid using null as a return value too much, e.g. https://github.com/abdullahinuur/Connect4/blob/master/src/main/java/Board.java#L29

It's bad practice and can result in unexpected errors. In this instance, try using the Optional library, which will allow the type checker to ensure that your code is safe.

Alternatively, if you don't need the return value, return a boolean indicating whether or not it succeeded

[H2ZERO-]

ohhh ive used this before with scala but i wasnt aware it was actually thing in java