0 
this.breakintoletters=()=>
        this.lengthi!==0?(this.title2=this.title,this.title2.split(),this.title2.
            map((x)=>this.arol.push(new letter(x))))
                        :!!false

So basically this is the code. It's supposed to break the string into letters and then push pertinent objects into an array.. Checks for the length of the string, if not 0 proceeds, returns an errors where MAP function is at...) TypeError telling me it's not a function. Editor is not showing errors. Would appreciate help

Improve this question
17
  • 2
    What an abuse of a ternary operator. And map. Commented Apr 7, 2019 at 17:58
  • 2
    Your code is highly unreadable, and map and push altogether is not good way to use map Commented Apr 7, 2019 at 17:59
  • 3
    "like it's NOT NOT FALSE hence true" - that's not how this booleanification works. It'll return false. Which is the original value to begin with. So using this trick here is pointless. Commented Apr 7, 2019 at 18:00
  • 1
    Your problem is this.title2.split() your not assigning splited value to this.title2 and than using map on this.title2 which is string Commented Apr 7, 2019 at 18:02
  • 1
    the splitting does not split anything without a value. Commented Apr 7, 2019 at 18:04

3 Answers 3

Reset to default
2

I suggest to use a different approach by checking this.lengthi in advance and return either false, or later the mapped length of the pushed values.

this.breakintoletters = () => {
    if (!this.lengthi) return false;
    this.title2 = this.title;
    // this.title2.split(); the result is not taken, not even splitted
    return this.title2.map(x => this.arol.push(new letter(x)));
};

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

You're not assigning splited value back to this.title2 and than using map on this.split2 which is a string

this.breakintoletters=()=>
        this.lengthi  ? (this.title2=this.title,
                         this.title2=this.title2.split(), 
                         this.title2.map((x)=>this.arol.push(new letter(x))))
                      :false

IMO you should try to make your code consice only upto a point where it stays readabale you can simply it in following manner

this.breakintoletters = ( ) => {
     if(this.lengthi === 0 ) return false;
     this.title2=this.title;
     this.title2=this.title2.split(); 
     return this.title2.map((x) => this.arol.push(new letter(x)))) 
 }
Improve this answer

Comments

0

(this.title2=this.title, this.title2.split(), this.title2. map((x)=>this.arol.push(new letter(x))))

Is this.arol the name of an array?

Try restructuring it to be:

(this.title.split().map((x) => this.arol.push(new letter(x)))

Methods like split() join() map() etc can be chained together.

I would rethink using the map function here though and the ternary which other commenters covered above. I mean it works technically, but if the goal is to iterate through the string in order to push certain values, it'd be better to use a for loop. Map functions are more when you want to iterate in order to apply the same specified method to each individual character

Also this is just a formatting thing but it makes it a lot easier to read and understand your code when you have some spaces between variables and operators, and choosing variable names that make sense for what you are doing (this.bookLength, this.reverseAr), or at least using the generic this.array or this.arr It makes it easier to ask questions like this because you'll get less clarifying questions about typos, and also if you are ever planning to work on a larger code base it's important to write clean code that is understandable to someone who doesn't know you

Improve this answer

3 Comments

Thank you very much for such detailed answer. this.lengthi is declared elsewhere, it's not 'length', I was doing !!false because one of my mentors is writing code like this (software developer), I just kind of copied this from him and ever since been using. By using !!false I was just trying to show that it would give false. this.arol is an array in the object, yeah. One of my mentors suggested using map/filter/reduce more often than for loops because it's easier to understand for other developers apparently. Opnion based I guess.
Map filter and reduce are great methods, but I explained a bit more above why it may not be the best choice in this case (specifically when iterrating for the purpose of pushing to an array it doesn't make much sense) but if it works it works. That is interesting about the !!false I have never seen that before in JavaScript, does your mentor work in JavaScript?
@MxOliver: !!false doesn't make any practical sense, of course, but this trick is widely used when you want to make sure you get a boolean and not another falsey value: answer = ""; !!answer (or something like that).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.