1
r-fu
5y

@highlight
disabled: Set<string> = new Set();

@highlight
<input
class="form-control"
id="pushBack"
type="text"
[attr.disabled]="disabled.has('pushBack') ? ' ' : null"
[(ngModel)]="local.pushBack"
/>

---
Do you know what's wrong with this code? If you know and you are a junior how would you tell this to your super boss without hurting his feelings?

Comments
  • 1
  • 1
  • 4
    Fuck ppl's feelings. Just don't be a cunt whenever you tell them what's wrong and if they get upset, that's their fault for being a pansy.
  • 1
    Have you tested it? Seems like it should work to disable, and then two way bind ngModel's underlying event emitter. My guess would be that he wants to have something that will maintain a collection of disabled values in the component at some point. It doesn't look like there's an input binding for the event, but I assume thats just copy pasta.

    Overall, it's kind of bush league. Reactive forms would make this infinitely better.
  • 1
    @SortOfTested hello. Thanks. First of all we dont have test suites. Hahaha. I was tasked to refactor the form and saw the disabled.has(), he’s using it to disabled the form but why the extra code? ‘ ‘ : null? When disabled.has() returns boolean and can be used to disable the form. Wanted to ask him but he might get mad. Just curious maybe i could learn something new because he has phd.
  • 2
    @Stuxnet fo real though. I’m afraid he might take it in a wrong way so ill just refactor this and add a comment. I just hope they will review my code. We dont have test and we dont review each others code which bothers me
  • 1
    @r-fu
    This is a pretty common practice stemming from a defect in Angular pre-version 4 where passing a boolean result to an expression affecting a Boolean attribute wouldn't work in IE browsers as it would attempt to assign the value rather than toggle. Might still be the case, I don't do that type of programming anymore.

    I would test your regressions and submit the fix if you care about it.
Add Comment