I don't like the Map version. My reason is basically that you're using a dynamic construct for a static purpose.
That translates to how easy it is to see what the code does. In the if-version, it's it's all right there and I know that when I read the code. When I read the Map-version, I need to find out where it's initialized and if I want to make sure that's all it does, I also need to make sure that no other code modifies the contents of the map.
Now, these problems can be minimized through developer discipline. However, especially when multiple people work on the same code or when the code and possibly the requirements change organically, discipline might lack in the end. Besides, if I'm new to your code, I don't know about the level of developer discipline, so I can't rely on it blindly.
I think your point on using the dynamic construct for a static purpose is a valid one. But I don't agree with the discipline part, in fact I think it's exactly the other way around.
First of all, this Map.of method returns an immutable map, so there's no way it could be modified after initialisation. No worries about that, then.
Second, what I've seen the most in my life is indisciplined uses of if statements, and I think that's the source of most of the bugs I have found, and the most difficulties to read and understand code.
I would have no problem with a group of disciplined developers using lots of if statements, as I know they would abide by good code constraints. For a team of less experienced developers, I prefer to have them work under constraints that will help them organize their code and ideas.
I did notice the Map.of and realized that it returned an immutable map, but since your post never mentions any programming language, I responded to the language-agnostic advice rather than the Java example. Not all languages can express such immutability, and it wasn't actually mentioned outside of the example either.
The other thing I meant to imply is that you have to keep the map initialization code close to the using the map. I think it's easy to forget the two are linked when you're writing unrelated code and you may end up placing that code between the initializing code and the using code, even when you're an experienced developer. It's not the kind of thing of thing that stands out in a code review either, I'd say. In my opinion, writing good ifs is about skill rather than discipline. Therefore, it's something people actually grow in. It's also something that stands out much more in a code review in my opinion.
(The difference I see between skill and discipline is that skill is a level you're at, closely related to experience, while discipline is a resource to be managed. A developer may or may not have different total discipline levels throughout their career, and they should probably learn to manage it better as they gain more experience, but that's not nearly as strongly coupled to experience, and there are always going to be moments of low discipline. On top of that, eliminating (or minimizing) the need for discipline wherever possible will free up more for where you do need it. I'm getting pretty philosophical here, though...)
When looking at the examples just now, there was something else that stood out to me. Note that this an observation without an opinion (it hasn't gestated enough to become an opinion yet) even if it may sound like an opinion. Both examples use enums. I used to like enums a lot a long time ago, but these days I don't use them much because I believe that in most cases they aren't the best tool for the job. Looking at the examples in that light, the second example looks to me like it would be better as three methods without arguments in which case there would neither be ifs nor a map. (The first example would still use the ifs or the map without the enum, I think.)
Thanks for the discussion so far! It's good to be able to debate things like this.
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
I don't like the Map version. My reason is basically that you're using a dynamic construct for a static purpose.
That translates to how easy it is to see what the code does. In the if-version, it's it's all right there and I know that when I read the code. When I read the Map-version, I need to find out where it's initialized and if I want to make sure that's all it does, I also need to make sure that no other code modifies the contents of the map.
Now, these problems can be minimized through developer discipline. However, especially when multiple people work on the same code or when the code and possibly the requirements change organically, discipline might lack in the end. Besides, if I'm new to your code, I don't know about the level of developer discipline, so I can't rely on it blindly.
Hi JasperHorn! Thanks for your feedback.
I think your point on using the dynamic construct for a static purpose is a valid one. But I don't agree with the discipline part, in fact I think it's exactly the other way around.
First of all, this Map.of method returns an immutable map, so there's no way it could be modified after initialisation. No worries about that, then.
Second, what I've seen the most in my life is indisciplined uses of if statements, and I think that's the source of most of the bugs I have found, and the most difficulties to read and understand code.
I would have no problem with a group of disciplined developers using lots of if statements, as I know they would abide by good code constraints. For a team of less experienced developers, I prefer to have them work under constraints that will help them organize their code and ideas.
Thanks for sharing!
Hi Thomaz,
I did notice the
Map.ofand realized that it returned an immutable map, but since your post never mentions any programming language, I responded to the language-agnostic advice rather than the Java example. Not all languages can express such immutability, and it wasn't actually mentioned outside of the example either.The other thing I meant to imply is that you have to keep the map initialization code close to the using the map. I think it's easy to forget the two are linked when you're writing unrelated code and you may end up placing that code between the initializing code and the using code, even when you're an experienced developer. It's not the kind of thing of thing that stands out in a code review either, I'd say. In my opinion, writing good ifs is about skill rather than discipline. Therefore, it's something people actually grow in. It's also something that stands out much more in a code review in my opinion.
(The difference I see between skill and discipline is that skill is a level you're at, closely related to experience, while discipline is a resource to be managed. A developer may or may not have different total discipline levels throughout their career, and they should probably learn to manage it better as they gain more experience, but that's not nearly as strongly coupled to experience, and there are always going to be moments of low discipline. On top of that, eliminating (or minimizing) the need for discipline wherever possible will free up more for where you do need it. I'm getting pretty philosophical here, though...)
When looking at the examples just now, there was something else that stood out to me. Note that this an observation without an opinion (it hasn't gestated enough to become an opinion yet) even if it may sound like an opinion. Both examples use enums. I used to like enums a lot a long time ago, but these days I don't use them much because I believe that in most cases they aren't the best tool for the job. Looking at the examples in that light, the second example looks to me like it would be better as three methods without arguments in which case there would neither be ifs nor a map. (The first example would still use the ifs or the map without the enum, I think.)
Thanks for the discussion so far! It's good to be able to debate things like this.