Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

refactor: simplify conditional assignment statements via ternary operator #5065

Closed
wants to merge 5 commits into from
Closed

Conversation

dimirc
Copy link
Contributor

@dimirc dimirc commented Nov 21, 2013

No description provided.

@petebacondarwin
Copy link
Contributor

The minification process actually does this sort of thing already. As an example here is the relevant section of minified code from $parse:

        if (g)
          "u" === h ?
            (h = this.text.substring(this.index + 1, this.index + 5), h.match(/[\da-f]{4}/i) || this.throwError("Invalid unicode escape [\\u" + h + "]"), this.index += 4, d += String.fromCharCode(parseInt(h, 16))) :
            d = (g = Td[h]) ?
              d + g :
              d + h,
            g = !1;

The main decision over making changes is whether it is more readable.

}
childBoundTranscludeFn = (afterTemplateNodeLinkFn.transclude)
? createBoundTranscludeFn(scope, afterTemplateNodeLinkFn.transclude)
: boundTranscludeFn;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler code is complex enough as it is. Spelling out the if statements here seems to me clearer.

@petebacondarwin
Copy link
Contributor

@dimirc - can you ensure you have signed the CLA. If you change the pull request please ensure that you follow the commit message guidelines, with a different commit for each file that you change: see https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md

Use ternary operator instead of if statement
Use ternary operator instead of if statement
Use ternary operator instead of if statement
Use logical OR operator instead of if statement
Use logical OR operator instead of if statement
@dimirc
Copy link
Contributor Author

dimirc commented Nov 21, 2013

@petebacondarwin commits changed. Btw, I signed CLA yesterday (name: Wladimir Coka)

@petebacondarwin
Copy link
Contributor

We can't find your CLA submission. Can you sign it again please?

@dimirc
Copy link
Contributor Author

dimirc commented Nov 27, 2013

@petebacondarwin ok no problem, I just signed CLA again with Sign Electronically option

@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.16, Backlog Jul 15, 2014
@IgorMinar
Copy link
Contributor

@petebacondarwin can you get this in please?

petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use ternary operator instead of if statement

Closes #5065
petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use ternary operator instead of if statement

Closes #5065
petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use logical OR operator instead of if statement

Closes #5065
petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use ternary operator instead of if statement

Closes #5065
petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use ternary operator instead of if statement

Closes #5065
petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use ternary operator instead of if statement

Closes #5065
petebacondarwin pushed a commit that referenced this pull request Jul 15, 2014
Use logical OR operator instead of if statement

Closes #5065
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Use ternary operator instead of if statement

Closes angular#5065
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Use ternary operator instead of if statement

Closes angular#5065
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Use ternary operator instead of if statement

Closes angular#5065
ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
Use logical OR operator instead of if statement

Closes angular#5065
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants