-
Notifications
You must be signed in to change notification settings - Fork 41
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
Shadow/Undeclared variables #159
Comments
no idea what those are and I suspect they are not beginner orientated content either. |
An instance variable is shadowed if a temporary variable have the same name. We can see that a lot when we load some code in Jenkins or with a Transcript open but there is almost no documentation about it. |
sounds like disaster coding to me... i rather advice people not to use same names for anything and keep their code as clear and readable as possible. This type of coding should not be allowed in the first place. |
This happen a lot. With hierarchy you don't always remember all the variables. And with configuration you can forget to define a dependency and you get Undeclared class/variables. You can see it on the console of a lot of builds: But if you do not know what is an undeclared/shadow variable/class you don't know what to do. |
But for me this is a bug and not a feature, and bugs should not be documented in tutorials they are meant to be removed. First of all you should not be allowed to create conflicts inside the system even if the system has priorities for those conflict., the system and the language must be clear or else we end up like C++. The same should apply when you remove a variable, but I dont see that as a problem so much because in that case like any language there should be an error saying that the variable does not exist. The first is a bug, the second should be a regular error. In any case thats my opinion, I am not the one to make such decision if this should be documented or not , but the way you describe it my opinion is that it should be not be documented because this way we allow these weaknesses to continue to exist and be accepted as normal and not being reported as what they are really are, bugs and bad designs that must get fixed. |
I tested your theory and it appears you are mistaken the system does not allow to define a temporary variable with the same name as an instance variable and it instead complains with the message "Name already defined". |
This is the case where you do it with a GUI without loading any code. But in headless or if you load some code this happen. |
But the opposite is allowed for example you can define a instance variable with the same name as an existing temporary variable, this is definitely a bug. If in headless you load code and this happens, is a bug, please report it. It should not happen. |
Since Monticello print it on the Transcript/stdout I think this is the expected behaviour. |
A variable can also be undefined if you run some code, delete a variable but the program is still running. |
Then let them brake, its still a bug, something is not stopping being a bug because if we fix it it brings up more bugs. Its a bug none the less. "A variable can also be undefined if you run some code, delete a variable but the program is still running." if the variable is removed then the debugger will complain that it does not exist , no ? why this should be documented ? its common sense. The same will happen with a method, or a class. If you remove something of course the system should complain , we talking here language fundamentals on error reporting. |
It's just that when people see "xxx is Undeclared" or "xxx is shadowed", if you don't know the problem, you don't know how to fix it. https://www.youtube.com/watch?v=VrTrQs5sDlA&feature=youtu.be |
I hear you loud and clear, but I want to be as clear as possible I dont want workarounds inside this book, I do not want documentation that allows bug and bad designs to exist. I want documnetation that teaches good designs and promotes the strenghs of Pharo. I rather have users complaining about their code braking on the mailing list so someone is motivated to fix these really bad designs than having this accepted as standard behavior. Again this is my opinion, if you want to add this to this tutorial I wont stop you but I wont accept the pulling request either because I believe is a dangerous mistake. But I wont stop anyone from accepting your pull request either. |
The goal is not to teach how to do bad design but in the contrary, how to keep the application good. A lot of people will not open a Transcript while loading there code or will not check what is a undeclared variable since it does not break an application most of the time. Maybe UPBE is not the right place for that but I think that this should be documented somewhere accessible because a lot of people don't know what they are and don't care if the application doesn't seems to break. |
I think that Cyril is 100% correct. |
Wow the fact that this discussion happened really bothers me. Freedom of expression is fundamental in any programming environment and fundamentally outweighs any philosophy you may have. It makes your environment perceived as rigid and makes it hostile to porting concepts from other environments into it. For example, if I want to copy over some concepts from COM, I won't be able to without lots of renaming to satisfy Pharo's obsession with case sensitivity(which is objectively has no reason to have); and if I want to copy over some concepts from functional programming, I won't be able to because once you name something, Pharo goes crazy when you try to shadow it. Pharo is an amazing environment if you are okay with only using dictionaries and blocks and only use classes/smalltalk object so you can set globals and persist state. But you can't get any real work done through the class browser because it has a very rigid and outdated philosophy on what is right and what is wrong that should be controlled by the user and their workgroup, not by pharo(unless you choose to limit yourself not to be able to register classes from others just because they happen to use shadowing or use lowercase for their class |
@dmitrymakhnin Hi, https://en.wikipedia.org/wiki/Coding_conventions
|
+1
…Sent from my iPhone
Encrypted email at [email protected]
On Mar 16, 2018, at 16:22, CyrilFerlicot ***@***.***> wrote:
@dmitrymakhnin Hi,
I don't have too much time to explain why I think naming convention are important but in the wikipedia pages there is a good start.
https://en.wikipedia.org/wiki/Coding_conventions
Reducing the cost of software maintenance is the most often cited reason for following coding conventions. In their introduction to code conventions for the Java programming language, Sun Microsystems provides the following rationale:
Code conventions are important to programmers for a number of reasons:
- 40%–80% of the lifetime cost of a piece of software goes to maintenance.
- Hardly any software is maintained for its whole life by the original author.
- Code conventions improve the readability of the software, allowing engineers to understand new code more quickly and thoroughly.
- If you ship your source code as a product, you need to make sure it is as well packaged and clean as any other product you create.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Most of this discussion is about documenting the convention not concretely explaining it.
Can you give a concrete example of this. These COM classes seem to conform to usual Pharo naming convention... https://msdn.microsoft.com/en-us/library/ms526615(v=exchg.10).aspx Also I don't believe there is anything concretely preventing you from using all uppercase classes. Object subclass: #ALLUPPERCASE ALLUPPERCASE >> test In Playground... Now doing... lowercase >> test does have a problem parsing... requiring...
By "goes crazy" are you just referring to Transcript output, or something else?
I believe it would be awfully hard to maintain a program with all-lowercase class names. |
All the way back to Pascal an inner scope variable can override the name of an outer scope variables. I don't think this is something we can realistically get away from allowing. But it is good practice to "report" such cases, thus good to document how to interpret such reports.
Its not a work-around. Shadowing is common across many languages. But we should help people understand the reporting. btw, perhaps it would be good to have a tool to "Find Shadow Variables" on the packages context menu, that would pop up list locations which were reported to the Transcript. |
Hi cyril We should ask marcus to write a little paragraph on Undeclared. because undeclared have nothing to do with shadowed variables. Undeclared is you load some code that refer to a class that does not exist or you remove a class that is referenced. The reference is turned into a first class so that we can manipulate after. Can we please this bug tracker about book issues? |
@jecisc it is important, but it is up to your workgroup to decide what is important in your workgroup, if you have one, not the vm by default. @bencoman you can have classes in all capitals, but you can't have classes start with lowercase(without commenting out the part of the validator that does the check, which is easy but can make installation a nightmare with this check forced by default). Likewise, you have to comment out parts of validator to allow methods to start with uppercase(which is a COMlike convention). By going crazy I mean this:
works fine but:
makes Transcript spam "UndefinedObject>>noMethod(n is shadowed)" and refuse to run. Furthermore, it prohibits you from using any key in |
I think that it would be interesting to explain what is a
shadow
and anundeclared
variable somewhere in UPBE because when I search for information about it some month ago I had a lot of trouble to find information.The text was updated successfully, but these errors were encountered: