Thread overview | |||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
July 11, 2013 Style question | ||||
---|---|---|---|---|
| ||||
I have a style question, because a friend of mine has a similar problem currently and I have no good advice for him. Let's assume we have this classes: ---- class MyClass { public: enum A { Foo = 0, Bar = 1 } private: A _a; public: this(A a) { this._a = a; } void test1() { MyStaticClass.test2(this._a); } } //---- enum B { Foo = 0, Bar = 1 } final abstract class MyStaticClass { public: static void test2(B b) { } } void main() { } ---- Prints: Error: function enum_problem.MyStaticClass.test2 (B b) is not callable using argument types (A) What should he do? As far as I can see he has 3 options: 1. An external file with the enum information. Both classes would import it and could use the same enum. But he cannot change the API, so this is no real option. 2. Change test1 into this: ---- void test1() { B b = cast(B) this.a; MyStaticClass.test2(b); } ---- This works fine. But is it safe? And is it good style? And how is this cast converted? Is it cheap? 3. Change test2 so that it accepts (even) (u)int. But then he lose the Type safety. Does anyone have any advice? |
July 11, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Namespace | On 2013-07-11, 20:22, Namespace wrote: > What should he do? > > As far as I can see he has 3 options: > 1. An external file with the enum information. Both classes would import it and could use the same enum. But he cannot change the API, so this is no real option. > > 2. Change test1 into this: > ---- > void test1() { > B b = cast(B) this.a; > MyStaticClass.test2(b); > } > ---- > This works fine. But is it safe? And is it good style? > And how is this cast converted? Is it cheap? > > 3. Change test2 so that it accepts (even) (u)int. But then he lose the Type safety. > > Does anyone have any advice? It seems to me that MyClass has access to MyStaticClass, and thus should also have access to B. If this is the case, why is MyClass using an A instead of a B? One option might be to use alias A = B; This would ensure the two enums are always the same. Last ditch, I would say option #2 is the best. It's not safe, in that if one enum changes and the other does not, the cast will blithely ignore that and use the same uint value. It is however efficient, as it is a simple reinterpretation of the bits. -- Simen |
July 11, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Simen Kjaeraas | > It seems to me that MyClass has access to MyStaticClass, and thus should > also have access to B. If this is the case, why is MyClass using an A > instead of a B? No, they are sadly not part of the same file / module. The // ---- should symbolize that. :D > One option might be to use alias A = B; This would ensure the two enums > are always the same. Yes that is a good idea. The only problem is, that he have doc comments on both enums. So, others which would use MyClass must then look into MyStaticClass to figure out what is supported. :/ That would be a bit annoying. > Last ditch, I would say option #2 is the best. It's not safe, in that > if one enum changes and the other does not, the cast will blithely > ignore that and use the same uint value. It is however efficient, as it > is a simple reinterpretation of the bits. Okay, I hoped for that. Would it be safe if I store A into an uint and cast this to B? Or is that the explicit variant what the compiler do implicit, if I use B b = cast(B) this.a; ? |
July 11, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Namespace | On 2013-07-11, 21:05, Namespace wrote: >> It seems to me that MyClass has access to MyStaticClass, and thus should >> also have access to B. If this is the case, why is MyClass using an A >> instead of a B? > > No, they are sadly not part of the same file / module. > The // ---- should symbolize that. :D Indeed, yet in MyClass.test1, you do reference MyStaticClass, which is indicated to be in the same file as B. But no matter. > Would it be safe if I store A into an uint and cast this to B? Or is that the explicit variant what the compiler do implicit, if I use B b = cast(B) this.a; ? uint tmp = A.Foo; B result = cast(B)tmp; gives the exact same result (and hopefully same asm) as casting directly from A to B. That also means that this compiles and gives (un?)expected results: uint tmp = 17; A a = cast(A)tmp; B b = cast(B)a; assert(b != B.Foo); assert(b != B.Bar); -- Simen |
July 11, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Namespace | On Thursday, 11 July 2013 at 18:22:11 UTC, Namespace wrote: > I have a style question, because a friend of mine has a similar problem currently and I have no good advice for him. > > Let's assume we have this classes: The whole situation looks strange. If you can change both files, than it is unclear what made you to write such inconsistent code. If you can change only one of them, then it should be adjusted to another (meaning importing external file and using that enum instead of trying to define different type and passing it). > ---- > class MyClass { > public: > enum A { > Foo = 0, > Bar = 1 > } > > private: > A _a; > public: > this(A a) { > this._a = a; > } > > void test1() { > MyStaticClass.test2(this._a); > } > } > > //---- > > enum B { > Foo = 0, > Bar = 1 > } > > final abstract class MyStaticClass { > public: > static void test2(B b) { } > } > > void main() { > > } > ---- > Prints: Error: function enum_problem.MyStaticClass.test2 (B b) is not callable using argument types (A) > > What should he do? Judging by "MyStaticClass.test2(this._a)" it seems that the first imports the second which is also suspicious - you are importing module which contains main function. From where does the code come from? > As far as I can see he has 3 options: > 1. An external file with the enum information. Both classes would import it and could use the same enum. But he cannot change the API, so this is no real option. > It would be good to clarify which file cannot be modified, although it does not really matter - just use one version of enum. > 2. Change test1 into this: > ---- > void test1() { > B b = cast(B) this.a; > MyStaticClass.test2(b); > } > ---- > This works fine. But is it safe? And is it good style? It is safe but not @safe (you will have complains if try to mark function as @safe). Of course this is a bad style, the whole case is strange. > And how is this cast converted? Is it cheap? Essentially nothing special is done. Of course it is cheap. > 3. Change test2 so that it accepts (even) (u)int. But then he lose the Type safety. This is almost same as direct cast in #2. Instead of making test2() accept B or (u)int consider making it static void test2 (MyClass.A a) and wipe out enum B entirely. |
July 11, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Maxim Fomin | > The whole situation looks strange. If you can change both files, than it is unclear what made you to write such inconsistent code. If you can change only one of them, then it should be adjusted to another (meaning importing external file and using that enum instead of trying to define different type and passing it). Besides the problem of changing existing API: If you change only one of those modules and use the enum from the other file, every user have to look into the file which declares the enum, to see which member the enum has. > Judging by "MyStaticClass.test2(this._a)" it seems that the first imports the second which is also suspicious - you are importing module which contains main function. From where does the code come from? It's my example of his problem. Only example code. > It would be good to clarify which file cannot be modified, although it does not really matter - just use one version of enum. That would be the best, I agree. > This is almost same as direct cast in #2. Instead of making test2() accept B or (u)int consider making it static void test2 (MyClass.A a) and wipe out enum B entirely. That's the problem I tried to describe above. If you have those two modules as in my example code, the one file without the enum depends on the one with the enum. Isn't that a bad situation? A possible solution would be to change one of those functions which accepts different enums, that it accept an uint - nothing else is an enum (in this case). |
July 12, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Namespace | On Thursday, 11 July 2013 at 20:15:52 UTC, Namespace wrote: >> The whole situation looks strange. If you can change both files, than it is unclear what made you to write such inconsistent code. If you can change only one of them, then it should be adjusted to another (meaning importing external file and using that enum instead of trying to define different type and passing it). > > Besides the problem of changing existing API: > If you change only one of those modules and use the enum from the other file, every user have to look into the file which declares the enum, to see which member the enum has. This argument can be applied to virtually any peice of code of any language which has import or #include. >> This is almost same as direct cast in #2. Instead of making test2() accept B or (u)int consider making it static void test2 (MyClass.A a) and wipe out enum B entirely. > > That's the problem I tried to describe above. > If you have those two modules as in my example code, the one file without the enum depends on the one with the enum. Isn't that a bad situation? Any file importing map depens on std.algorithm. Isn't that a bad situation? > A possible solution would be to change one of those functions which accepts different enums, that it accept an uint - nothing else is an enum (in this case). Well if you consider that this is a best option nothing stops you from doing this. In such case you can't mark functions as @safe and may run into bugs when some integer value placed into enum does not fit into allowed values. This problem is tackled by final switches. By the way, in your case base type is int, not uint. |
July 12, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Namespace | On 2013-07-11 20:22, Namespace wrote: > [snip] > Does anyone have any advice? Who is supposed to access these enums? Can you put it in a separate module but in the same package with package protection? -- /Jacob Carlborg |
July 12, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jacob Carlborg | On Friday, 12 July 2013 at 06:54:40 UTC, Jacob Carlborg wrote:
> On 2013-07-11 20:22, Namespace wrote:
>> [snip]
>> Does anyone have any advice?
>
> Who is supposed to access these enums? Can you put it in a separate module but in the same package with package protection?
No. Both modules are in different packages, so the external enum module must be placed in one of them.
It seems that he decided to use the cast version to avoid dependencies.
Thanks for your help. :)
|
July 12, 2013 Re: Style question | ||||
---|---|---|---|---|
| ||||
Posted in reply to Namespace | On Thu, 11 Jul 2013 19:22:10 +0100, Namespace <rswhite4@googlemail.com> wrote: > I have a style question, because a friend of mine has a similar problem currently and I have no good advice for him. > > Let's assume we have this classes: > > ---- > class MyClass { > public: > enum A { > Foo = 0, > Bar = 1 > } > > private: > A _a; > public: > this(A a) { > this._a = a; > } > > void test1() { > MyStaticClass.test2(this._a); > } > } > > //---- > > enum B { > Foo = 0, > Bar = 1 > } > > final abstract class MyStaticClass { > public: > static void test2(B b) { } > } > > void main() { > > } > ---- > Prints: Error: function enum_problem.MyStaticClass.test2 (B b) is not callable using argument types (A) > > What should he do? If A and B are supposed to be/represent the same thing, then they should be the same enumeration - move them/it into a separate module and import into both MyClass and MyStaticClass. If they're supposed to be different, then you treat them as separate types and either cast, or range check then cast. So, add a uint constructor to MyStaticClass (in addition to the existing B constructor). Have the uint constructor range check the value using assert, or exceptions, then cast valid values to B internally. R -- Using Opera's revolutionary email client: http://www.opera.com/mail/ |
Copyright © 1999-2021 by the D Language Foundation