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

Better typing on Arrays. #25

Closed
wants to merge 1 commit into from
Closed

Conversation

BigDubb
Copy link

@BigDubb BigDubb commented Oct 21, 2015

Added typing to array of palettes. This makes understanding which palette you're working with Bright vs. Normal.
Moved the creation of the palette to process_ansi method. This method is the only method that uses palette_256. Why pay the price for code that may never be executed. There are several public methods available on your class that dont utilize this object.

Added typing to array of palettes.  This makes understanding which palette you're working with Bright vs. Normal.  
Moved the creation of the palette to process_ansi method. This method is the only method that uses palette_256. Why pay the price for code that may never be executed. There are several public methods available on your class that dont utilize this object.
@BigDubb
Copy link
Author

BigDubb commented Oct 23, 2015

I dont know if I agree with the first assessment on the issue. I can create a new class and once instantiated the class and the methods on the class are executable without having to new up a class.

The request for getting at the palettes is like your Set/Get approach. If you're going to make those properties available in that methodology no need to make the properties public.

The O in SOLID. Classes are open to extension but closed to modification.

By opening up the doors and making everything public you start to loose control over what's going on inside the class. local vars like bg and fg are public ally exposed an they should never be. Bright is also exposed, and that's definitely a private value. AGain only used in the process_Ansi method.

General rule, Make your classes only expose what they need to expose to make them useable/consumable. Dont expose your classes too much or they start to become difficult to manage if other items from outside the object can manipulate its innards.

The request of exposing the ansi colors starts to run into some issues. 1. the example given starts to change the signature of the object via duckputting. Not good in strongly typed libraries.

If you just exposed AnsiColors and just allowed a user to modify class names, that would work. HOWEVER, you run into some issues where a consumer might start removing items from the array and then screw up other aspects of the tool. In order to go off the key value you'll have to test if the object exists then handle stuff elsewise. It will start to get unnecessarily complicated.

I would....

Make AUColor a clas that implements the appropriate interface, Then expose a method on it to Set the CSS Class name.

So it might be
au.AnsiColors.NormalColors[i].ClassName = "Foo_class".

Only the AuColor cares about its own CSSclass and only it can manipulate its own CSSclass.

Encapsulation principal of OOD.

@drudru
Copy link
Owner

drudru commented Apr 22, 2018

This is old now. So closing.

@drudru drudru closed this Apr 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants