Question

I'm making a 2 player chess game which involves a lot of logic. It's the first time I've written a program that involves a lot of logic, inheritance, and code organization. I find myself running into a lot of spaghetti code and functions that don't limit themselves to doing one thing like I know is good practice. I've tried to think of ideas to limit my spaghetti code but am uncertain of the best way to go about it. Here's an example of something I run into a lot.

if (that.selected && that.selected != target) {
  if (that.positions[target] && that.positions[target].color == piece.color) {
      ...
  } else {
    var ind = xyz...;
    if (piece.type == "king" && ind != -1) {
    } else {
    ...
    }
  }

My best idea thus far for reducing this is to move the conditionals into separate functions (for example, instead of the first line of my posted code I could do something like...

var selected_non_target = selectedNonTarget(that.selected, target);
if (selected_non_target) {
  continue above code;
}
function selectedNonTarget(selected, target){
  if (selected && selected != target) {
    ... maybe do some stuff ... 
    return true;
  }
}

But I don't really see how this would help and is much more code... Here's the repository. I would love any suggestions. https://github.com/natecraft1/javascript_chess/blob/master/index.html

Was it helpful?

Solution

I think your own suggestion is a good start. You can reduce it a little bit -- no need for the first var:

if (selectedNonTarget(that.selected, target))
{   
    // continue above code; 
} 

function selectedNonTarget(selected, target){
    // return a bool -- resist the urge to do other things here   
    return (selected && selected != target); 
}

This is good coding practice to reduce a compound condition to a well-named function and then treat it like a simple true | false in your main logic. This should help reduce the cognitive load as your code base grows -- and every little bit helps.

Sure we all understand what && and != are supposed to mean and do but at 2am after 12 hours of coding, you'll be less error-prone with if (selectedNonTarget(that.selected, target))

I would also strongly recommend spending some time splitting the 1000 or so lines of code you have into multiple files. You will find this worthwhile.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top