Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use common prefix for high-R/high-V circuit elements #936

Closed
arouinfar opened this issue Jan 18, 2023 · 6 comments
Closed

Use common prefix for high-R/high-V circuit elements #936

arouinfar opened this issue Jan 18, 2023 · 6 comments

Comments

@arouinfar
Copy link
Contributor

Related to #917

We currently use highResistance and highVoltage as prefixes for associated resistors, bulbs, and batteries. In addition to being verbose, these prefixes aren't always accurate. A standard bulb (max R = 120 Ω) can have a larger resistance than a high-R bulb (min R = 100 Ω). A common prefix would simplify the language and result in these elements being grouped together in the tree.

In discussions with @matthew-blackman and @samreid, we tentatively liked "extreme". @samreid please proceed with this change. If you think of any alternatives to "extreme" please let me know. I think it evokes the right idea, but there may be a better term we haven't thought of yet.

@samreid
Copy link
Member

samreid commented Jan 19, 2023

It seems anyone could help on this. We want to rename the any tandems beginning with highResistance* or highVoltage* with extreme*. Internal code details (variable names, methods, parameter names, etc should be updated as well). Maybe even class names? Not sure.

@samreid
Copy link
Member

samreid commented Jan 20, 2023

This looks good to me, nice work. I'm wondering if we will want to change extreme to isExtreme for the options parameters and class attributes. I was about to recommend that, when I saw that we already have LightBulb.real instead of LightBulb.isReal, but now I'm feeling we should change both. But I looked in the code review checklist, and did not see a convention for it. @matthew-blackman what do you recommend?

@samreid samreid assigned matthew-blackman and unassigned samreid Jan 20, 2023
@matthew-blackman
Copy link

I like the prefix 'is' as a style pattern for boolean options. Will update 'extreme' to 'isExtreme' and 'real' to 'isReal'.

@samreid
Copy link
Member

samreid commented Feb 2, 2023

Excellent, looks great, thanks, closing.

@samreid samreid closed this as completed Feb 2, 2023
@samreid
Copy link
Member

samreid commented Feb 2, 2023

I also noticed a constant that looks like it should be renamed to match:

const isExtreme = selectedCircuitElement.resistorType === ResistorType.HIGH_RESISTANCE_RESISTOR;

Perhaps rename ResistorType.HIGH_RESISTANCE_RESISTOR => ResistorType.EXTREME_RESISTOR?

@samreid
Copy link
Member

samreid commented Feb 3, 2023

Fixed, closing.

@samreid samreid closed this as completed Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants