2
\$\begingroup\$

I'm developing, just for fun and learning, a TypeScript Class that creates an Array<Array<string>> that represents a pyramid. So, for length = 5 the array would look like :

[
  ['X', 'X', '0', 'X', 'X'],
  ['X', '0', '0', '0', 'X'],
  ['0', '0', '0', '0', '0']
]

The class constructor can take a few arguments:

  1. The final array.
  2. The length (width) of the pyramid, the "blank" String and the "piece" string

My implementation looks like:

PLUNKER

interface IPiramide {
    array?          : Array<Array<string>>,
    private blanco? :         string = 'X',
    private trozo?  :         string = 'O'
}

class Piramide implements IPiramide {

  constructor(piramide : IPiramide = {blanco : 'X', trozo : '0', ancho : 0}){

    this.blanco = piramide.blanco;
    this.trozo = piramide.trozo;

        if(piramide.array){
            this.array = piramide.array;
        }
        else if(piramide.ancho){
            this.build(piramide.ancho);
        }

    }

    static getInstance(ancho : number, blanco : string, trozo : string, inverse ? : boolean) : Piramide {
        return new Piramide({blanco, trozo}).build(ancho, inverse);
    }

    private build (ancho : number, inverse? : boolean = false) {
        this.array = [];
        let numberCount = ancho % 2 == 0 ? 2 : 1;
        let middle = Math.floor(ancho / 2);

        while (numberCount != ancho + 2){
            this.array.push(Array.from({length : ancho}, (el, idx)=>{
                let range = (ancho - numberCount)/2;
                return (idx < range || idx > ancho - range - 1) 
                  ? this.blanco 
                  : this.trozo;
            }));
            numberCount += 2;
        }
        inverse 
          ? this.array = this.array.map((el, idx, arr)=>arr[arr.length - idx - 1]) 
          : this.array;
        return this;
    }

    public getArray (){
        return this.array;
    }

    public getTrozo() {
      return this.array[Math.floor(this.array.length/2)][Math.floor(this.ancho/2)];
    }

    public getBlanco(){
      if(this.array.length){
        return this.array[1][0];
      }
      return null;
    }

    public getAncho(){
      return this.array[0].length;
    }

    public getAlto(){
      return this.array.length;
    }

    toString(){
        return this.array.map( el=> el.join(' ')).join('\n');
    }

    isConcatenable (piramide : Piramide) : boolean {
      console.log('isConcat', this);
      return this.getAlto() == piramide.getAlto();
    }

    reverse() : Piramide{
      this.array = this.array.map( (el, idx, arr) => arr[arr.length - idx - 1]);
      return this;
    }

    concatRight (...piramides : Array<Piramide>) : Piramide {
      return new Piramide({
        array : this.array.map( (el, i)=> el.concat(...piramides.filter(this.isConcatenable.bind(this)).map( pir => pir.getArray()[i] ) ) ),
        blanco : this.blanco, 
        trozo : this.trozo
      });     
    }

    concatLeft (...piramides : Array<Piramide>) : Piramide {
      return new Piramide({
        array :  this.array.map( (el, i)=> [].concat(...piramides.filter(this.isConcatenable.bind(this)).map( pir => pir.getArray()[i] ) ) ).map( (el, i)=> el.concat(this.array[i])),
        blanco : this.blanco, 
        trozo : this.trozo
      });     
    }

    concatAbove(...piramides : Array<Piramide>) {
      return new Piramide({
        array : piramides.reduce( (arr, pir) => arr.concat(pir.getArray()) , []).concat(this.array),
        blanco : this.blanco, 
        trozo : this.trozo
      });     
    }

    concatBelow(...piramides : Array<Piramide>) {
      return new Piramide({
        array : this.array.concat(piramides.reduce( (arr, pir) => arr.concat(pir.getArray()) , [])),
        blanco : this.blanco, 
        trozo : this.trozo
      });     
    }

}

It works, but it seems to me that this could be improved (I feel weird with my constructor and static methods).

Could this code be improved?

\$\endgroup\$
2
\$\begingroup\$

When writing a simple function to return something if something is true or return something else, you should probably only have one return statement in the function.

increasing the amount of returns in a function makes the function less readable and more prone to mistakes. if you only need one return statement, then only use one return statement.

Personally I would write this

public getBlanco(){
  if(this.array.length){
    return this.array[1][0];
  }
  return null;
}

like this instead

public getBlanco(){
    return this.array.length ? this.array[1][0] : null;
}

using a ternary is really the cleaner way to do this because you are really saying

if there are items in the array
    return item [1][0]
else
    return null
\$\endgroup\$

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.