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

Need for namespace reset override #1315

Open
TimLethbridge opened this Issue Jun 23, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@TimLethbridge
Member

TimLethbridge commented Jun 23, 2018

Currently if you say

namespace n;
class X {}

namespace m;
class Y {}

class X{}

you get in UmpleOnline

Warning on line 7 : Redefining the namespace of class X to m. More information (30)
Warning on line 9 : Redefining the namespace of class X to m. More information (30)

There are two problems here:

  1. You might want in the second redefinition to not redefine the namespace to m. This is a serious problem when using a new file or mixset.

  2. In UmpleOnline the second message appears due to the hidden diagram information. The warning and redefinition will always appear.

Solution alternatives

a) Reverse the situation. Only take the first namespace, and leave it in place. Also add a 'namespace -;' syntax that would indicate that following this point no namespace is active. That could be injected to step putting subsequent classes in a namespace. This could be injected at the start of the diagram code.

b) Add an extra optional second argument to the namespace directive 'final' (or something similar) to prevent overrides and warnings about them.

c) Leave the first namespace as the final namespace. Don't override on any redefinition by default, and hence drop the warning. Also add a 'override' as a possible second argument to force an override should this be desired. The idea of having namespace - would still be a good idea but would not be needed to be injected in UmpleOnline for diagrams.

Suggest option c

@vahdat-ab

This comment has been minimized.

Member

vahdat-ab commented Jun 25, 2018

The issue you opened is one of the issues I have faced a while ago and tried to fix this with some constraints and warning. There are many scenarios you may imagine to mix namespaces. It becomes more complicated when you use use statements that have several namespaces defined inside the used files. Even the order of using statement will cause some concerns (with respect to other use statements or other names spaces)
The person who wants to fix this issue must also consider these cases.
Fixing this issue with new definitions may cause some problems in building UmpleCompiler because there are some dependencies about how use statement works.

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Jun 25, 2018

OK. So here is the proposed solution, reflecting Vahdat's issue and respecting the need to enable existing systems to still compile.

  1. Add 'final' as an optional second argument to the namespace directive. This prevents classes set with this namespace directive from being reset by a later mixin or mixset triggered by a use statement.

  2. Add '-' as an optional argument to the namespace directive. This cancels any outstanding namespace. Subsequently defined classes default to no namespace (unless a redefinition is encountered in a subsequent mixin/mixset for them. Note that '- final' makes no sense.

  3. Add a warning if an attempt to redefine a final namespace is encountered (saying it is ignored), The existing warning would still be in place for non-final namespace redefinition.

  4. Document as follows.

  • Best practice is to specify 'namespace X final' before a group of classes to be in the namespace and then 'namespace -' after the block. This could even be done in a mixin to structure the system.
@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Sep 5, 2018

See also #52

@TimLethbridge

This comment has been minimized.

Member

TimLethbridge commented Sep 23, 2018

In fact although the warning messages 30 say that the namespace is being redefined, in fact it it is not.

also if you attempt to specify the original namespace, you still get the warning:

namespace n;
class X {}

namespace m;
class Y {}


namespace n;
class X{}

Therefore the best solution is

  1. Change the warning message to say, 'Attempt to redefine ...'. Assume by default all initial namespaces are redefined.

  2. Remove the warning in the case above in this comment, when it is actually not changing what was previously set.

  3. Add an --override that would allow changing of the namespace in a mixin e.g.

namespace n;
class X {}

namespace m;
class Y {}

namespace p --override;
class X{}

In this case, class X would end up being in p not in n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment