Get the highest role of a user across all the organizations he is member of
up vote
1
down vote
favorite
I have a function that looks like this:
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Admin)) {
Some(OrganizationMembership.Level.Admin)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Member)) {
Some(OrganizationMembership.Level.Member)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Guest)) {
Some(OrganizationMembership.Level.Guest)
} else {
None
}
}
The gets the highest role of a user across all the organizations he is member of. As you can see, the function is pretty straightforward and works. However, it has a complexity of O(n3). I was wondering if there is a way to get it to O(n) or at least a bit lower?
performance scala
bumped to the homepage by Community♦ 2 days ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
add a comment |
up vote
1
down vote
favorite
I have a function that looks like this:
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Admin)) {
Some(OrganizationMembership.Level.Admin)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Member)) {
Some(OrganizationMembership.Level.Member)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Guest)) {
Some(OrganizationMembership.Level.Guest)
} else {
None
}
}
The gets the highest role of a user across all the organizations he is member of. As you can see, the function is pretty straightforward and works. However, it has a complexity of O(n3). I was wondering if there is a way to get it to O(n) or at least a bit lower?
performance scala
bumped to the homepage by Community♦ 2 days ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
I don't see the $mathcal O(n^3)$ complexity. I only see $mathcal O(3n)$, which by definition is equivalent to $mathcal O(n)$.
– Roland Illig
Oct 17 '17 at 6:46
hmmm... You are so right! I am usually not that slow. )))) Sorry..
– Shurik Agulyansky
Oct 17 '17 at 6:50
It was a fun exercise though :)
– Shurik Agulyansky
Oct 17 '17 at 6:50
add a comment |
up vote
1
down vote
favorite
up vote
1
down vote
favorite
I have a function that looks like this:
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Admin)) {
Some(OrganizationMembership.Level.Admin)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Member)) {
Some(OrganizationMembership.Level.Member)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Guest)) {
Some(OrganizationMembership.Level.Guest)
} else {
None
}
}
The gets the highest role of a user across all the organizations he is member of. As you can see, the function is pretty straightforward and works. However, it has a complexity of O(n3). I was wondering if there is a way to get it to O(n) or at least a bit lower?
performance scala
I have a function that looks like this:
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Admin)) {
Some(OrganizationMembership.Level.Admin)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Member)) {
Some(OrganizationMembership.Level.Member)
} else if (organizationMemberships.exists(_.level == OrganizationMembership.Level.Guest)) {
Some(OrganizationMembership.Level.Guest)
} else {
None
}
}
The gets the highest role of a user across all the organizations he is member of. As you can see, the function is pretty straightforward and works. However, it has a complexity of O(n3). I was wondering if there is a way to get it to O(n) or at least a bit lower?
performance scala
performance scala
edited Oct 17 '17 at 0:15
200_success
127k15148411
127k15148411
asked Oct 16 '17 at 23:21
Shurik Agulyansky
1084
1084
bumped to the homepage by Community♦ 2 days ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
bumped to the homepage by Community♦ 2 days ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
I don't see the $mathcal O(n^3)$ complexity. I only see $mathcal O(3n)$, which by definition is equivalent to $mathcal O(n)$.
– Roland Illig
Oct 17 '17 at 6:46
hmmm... You are so right! I am usually not that slow. )))) Sorry..
– Shurik Agulyansky
Oct 17 '17 at 6:50
It was a fun exercise though :)
– Shurik Agulyansky
Oct 17 '17 at 6:50
add a comment |
I don't see the $mathcal O(n^3)$ complexity. I only see $mathcal O(3n)$, which by definition is equivalent to $mathcal O(n)$.
– Roland Illig
Oct 17 '17 at 6:46
hmmm... You are so right! I am usually not that slow. )))) Sorry..
– Shurik Agulyansky
Oct 17 '17 at 6:50
It was a fun exercise though :)
– Shurik Agulyansky
Oct 17 '17 at 6:50
I don't see the $mathcal O(n^3)$ complexity. I only see $mathcal O(3n)$, which by definition is equivalent to $mathcal O(n)$.
– Roland Illig
Oct 17 '17 at 6:46
I don't see the $mathcal O(n^3)$ complexity. I only see $mathcal O(3n)$, which by definition is equivalent to $mathcal O(n)$.
– Roland Illig
Oct 17 '17 at 6:46
hmmm... You are so right! I am usually not that slow. )))) Sorry..
– Shurik Agulyansky
Oct 17 '17 at 6:50
hmmm... You are so right! I am usually not that slow. )))) Sorry..
– Shurik Agulyansky
Oct 17 '17 at 6:50
It was a fun exercise though :)
– Shurik Agulyansky
Oct 17 '17 at 6:50
It was a fun exercise though :)
– Shurik Agulyansky
Oct 17 '17 at 6:50
add a comment |
2 Answers
2
active
oldest
votes
up vote
0
down vote
Well, this is what I did. Not sure if that's the best way to go.
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.nonEmpty) {
val levelMap = Map(
OrganizationMembership.Level.Guest -> 0,
OrganizationMembership.Level.Member -> 1,
OrganizationMembership.Level.Admin -> 2
)
val maxLevel = organizationMemberships.foldLeft(OrganizationMembership.Level.Guest) {
(level, organizationMembership) =>
if (levelMap(organizationMembership.level) > levelMap(level)) {
organizationMembership.level
} else {
level
}
}
Some(maxLevel)
} else {
None
}
}
The idea behind the code above, is the fact that we are iterating through the collection only once, regardless of the collection size.
The levelMap was created strictly for the reason of easier comparison.
The cool thing about foldLeft in this case is that we don't need to create mutable variables while iterating through the collection and it is actually very short and quite readable code.
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
1
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
add a comment |
up vote
0
down vote
It looks like what you're doing could be expressed thusly.
import scala.util.Try
val levelMap = Map(...
Try(getAllByUserId(userId).maxBy(org => levelMap(org.level)).level).toOption
It's worth noting that this will also catch any exceptions thrown by levelMap. (Maybe a new level is encountered not yet included in levelMap.) That might or might not be considered a good thing.
You can unpack it some to avoid the try/catch overhead hidden inside the Try type.
if (organizationMemberships.isEmpty)
None
else
Some(organizationMemberships.maxBy(om => levelMap(om.level)).level)
add a comment |
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
0
down vote
Well, this is what I did. Not sure if that's the best way to go.
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.nonEmpty) {
val levelMap = Map(
OrganizationMembership.Level.Guest -> 0,
OrganizationMembership.Level.Member -> 1,
OrganizationMembership.Level.Admin -> 2
)
val maxLevel = organizationMemberships.foldLeft(OrganizationMembership.Level.Guest) {
(level, organizationMembership) =>
if (levelMap(organizationMembership.level) > levelMap(level)) {
organizationMembership.level
} else {
level
}
}
Some(maxLevel)
} else {
None
}
}
The idea behind the code above, is the fact that we are iterating through the collection only once, regardless of the collection size.
The levelMap was created strictly for the reason of easier comparison.
The cool thing about foldLeft in this case is that we don't need to create mutable variables while iterating through the collection and it is actually very short and quite readable code.
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
1
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
add a comment |
up vote
0
down vote
Well, this is what I did. Not sure if that's the best way to go.
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.nonEmpty) {
val levelMap = Map(
OrganizationMembership.Level.Guest -> 0,
OrganizationMembership.Level.Member -> 1,
OrganizationMembership.Level.Admin -> 2
)
val maxLevel = organizationMemberships.foldLeft(OrganizationMembership.Level.Guest) {
(level, organizationMembership) =>
if (levelMap(organizationMembership.level) > levelMap(level)) {
organizationMembership.level
} else {
level
}
}
Some(maxLevel)
} else {
None
}
}
The idea behind the code above, is the fact that we are iterating through the collection only once, regardless of the collection size.
The levelMap was created strictly for the reason of easier comparison.
The cool thing about foldLeft in this case is that we don't need to create mutable variables while iterating through the collection and it is actually very short and quite readable code.
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
1
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
add a comment |
up vote
0
down vote
up vote
0
down vote
Well, this is what I did. Not sure if that's the best way to go.
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.nonEmpty) {
val levelMap = Map(
OrganizationMembership.Level.Guest -> 0,
OrganizationMembership.Level.Member -> 1,
OrganizationMembership.Level.Admin -> 2
)
val maxLevel = organizationMemberships.foldLeft(OrganizationMembership.Level.Guest) {
(level, organizationMembership) =>
if (levelMap(organizationMembership.level) > levelMap(level)) {
organizationMembership.level
} else {
level
}
}
Some(maxLevel)
} else {
None
}
}
The idea behind the code above, is the fact that we are iterating through the collection only once, regardless of the collection size.
The levelMap was created strictly for the reason of easier comparison.
The cool thing about foldLeft in this case is that we don't need to create mutable variables while iterating through the collection and it is actually very short and quite readable code.
Well, this is what I did. Not sure if that's the best way to go.
def getMaxUserLevelByUserId(userId: Muid)(
implicit connection: Connection): Option[OrganizationMembership.Level.Value] = {
val organizationMemberships = getAllByUserId(userId)
if (organizationMemberships.nonEmpty) {
val levelMap = Map(
OrganizationMembership.Level.Guest -> 0,
OrganizationMembership.Level.Member -> 1,
OrganizationMembership.Level.Admin -> 2
)
val maxLevel = organizationMemberships.foldLeft(OrganizationMembership.Level.Guest) {
(level, organizationMembership) =>
if (levelMap(organizationMembership.level) > levelMap(level)) {
organizationMembership.level
} else {
level
}
}
Some(maxLevel)
} else {
None
}
}
The idea behind the code above, is the fact that we are iterating through the collection only once, regardless of the collection size.
The levelMap was created strictly for the reason of easier comparison.
The cool thing about foldLeft in this case is that we don't need to create mutable variables while iterating through the collection and it is actually very short and quite readable code.
edited Oct 17 '17 at 5:20
answered Oct 16 '17 at 23:59
Shurik Agulyansky
1084
1084
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
1
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
add a comment |
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
1
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
Self answers are allowed, but code dumps (with no explanation of the motivations behind your changes) are not valid answers. Furthermore, if you are seeking a review of this revised code, then you should make it the new question instead (assuming that no other answers have been posted in the meantime).
– 200_success
Oct 17 '17 at 0:18
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
I apologize, and agree. I had about 2 minutes to do it before I had to run. I will put an explanation a bit later. I am not seeking review of my answer though... I was seeking for an idea of original question
– Shurik Agulyansky
Oct 17 '17 at 0:21
1
1
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
@200_success Here you go. I updated the answer with more explanation.
– Shurik Agulyansky
Oct 17 '17 at 5:21
add a comment |
up vote
0
down vote
It looks like what you're doing could be expressed thusly.
import scala.util.Try
val levelMap = Map(...
Try(getAllByUserId(userId).maxBy(org => levelMap(org.level)).level).toOption
It's worth noting that this will also catch any exceptions thrown by levelMap. (Maybe a new level is encountered not yet included in levelMap.) That might or might not be considered a good thing.
You can unpack it some to avoid the try/catch overhead hidden inside the Try type.
if (organizationMemberships.isEmpty)
None
else
Some(organizationMemberships.maxBy(om => levelMap(om.level)).level)
add a comment |
up vote
0
down vote
It looks like what you're doing could be expressed thusly.
import scala.util.Try
val levelMap = Map(...
Try(getAllByUserId(userId).maxBy(org => levelMap(org.level)).level).toOption
It's worth noting that this will also catch any exceptions thrown by levelMap. (Maybe a new level is encountered not yet included in levelMap.) That might or might not be considered a good thing.
You can unpack it some to avoid the try/catch overhead hidden inside the Try type.
if (organizationMemberships.isEmpty)
None
else
Some(organizationMemberships.maxBy(om => levelMap(om.level)).level)
add a comment |
up vote
0
down vote
up vote
0
down vote
It looks like what you're doing could be expressed thusly.
import scala.util.Try
val levelMap = Map(...
Try(getAllByUserId(userId).maxBy(org => levelMap(org.level)).level).toOption
It's worth noting that this will also catch any exceptions thrown by levelMap. (Maybe a new level is encountered not yet included in levelMap.) That might or might not be considered a good thing.
You can unpack it some to avoid the try/catch overhead hidden inside the Try type.
if (organizationMemberships.isEmpty)
None
else
Some(organizationMemberships.maxBy(om => levelMap(om.level)).level)
It looks like what you're doing could be expressed thusly.
import scala.util.Try
val levelMap = Map(...
Try(getAllByUserId(userId).maxBy(org => levelMap(org.level)).level).toOption
It's worth noting that this will also catch any exceptions thrown by levelMap. (Maybe a new level is encountered not yet included in levelMap.) That might or might not be considered a good thing.
You can unpack it some to avoid the try/catch overhead hidden inside the Try type.
if (organizationMemberships.isEmpty)
None
else
Some(organizationMemberships.maxBy(om => levelMap(om.level)).level)
answered Nov 27 '17 at 3:04
jwvh
51626
51626
add a comment |
add a comment |
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f178084%2fget-the-highest-role-of-a-user-across-all-the-organizations-he-is-member-of%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
I don't see the $mathcal O(n^3)$ complexity. I only see $mathcal O(3n)$, which by definition is equivalent to $mathcal O(n)$.
– Roland Illig
Oct 17 '17 at 6:46
hmmm... You are so right! I am usually not that slow. )))) Sorry..
– Shurik Agulyansky
Oct 17 '17 at 6:50
It was a fun exercise though :)
– Shurik Agulyansky
Oct 17 '17 at 6:50