Count number of gemstones in a set of minerals

up vote
4
down vote

favorite

This is the "Gemstones" problem on Hackerrank.

John has collected various rocks. Each rock has various minerals embeded in it. Each type of mineral is designated by a lowercase letter in the range a-z. There may be multiple occurrences of a mineral in a rock. A mineral is called a gemstone if it occurs at least once in each of the rocks in John's collection.
Given a list of minerals embedded in each of John's rocks, display the number of types of gemstones he has in his collection.
For example, the array of mineral composition strings [abc, abc, bc] The minerals b and c appear in each composite, so there are 2 gemstones.

This is my Python code:

def gemstones(arr): 
 for i in range(len(arr)):
 arr[i] = "".join(set(arr[i]))

 long = max(arr, key=len)
 arrlen = len(arr)
 flag,count = 0,0
 for i in long: 
 for j in range(arrlen):
 if i not in arr[j]: flag = 1
 if flag is 0: count += 1
 flag = 0
 return count

Are there ways to improve this code? I feel I'm not using the full functionality of Python.

A test case:

>>> arr = ['abcdde', 'baccd', 'eeabg']
>>> print(gemstones(arr))
2

edited 1 hour ago

Mathias Ettinger

22.1k32976

asked 1 hour ago

db18

814

add a commentÂ |Â

up vote
4
down vote

favorite

This is the "Gemstones" problem on Hackerrank.

John has collected various rocks. Each rock has various minerals embeded in it. Each type of mineral is designated by a lowercase letter in the range a-z. There may be multiple occurrences of a mineral in a rock. A mineral is called a gemstone if it occurs at least once in each of the rocks in John's collection.
Given a list of minerals embedded in each of John's rocks, display the number of types of gemstones he has in his collection.
For example, the array of mineral composition strings [abc, abc, bc] The minerals b and c appear in each composite, so there are 2 gemstones.

This is my Python code:

def gemstones(arr): 
 for i in range(len(arr)):
 arr[i] = "".join(set(arr[i]))

 long = max(arr, key=len)
 arrlen = len(arr)
 flag,count = 0,0
 for i in long: 
 for j in range(arrlen):
 if i not in arr[j]: flag = 1
 if flag is 0: count += 1
 flag = 0
 return count

Are there ways to improve this code? I feel I'm not using the full functionality of Python.

A test case:

>>> arr = ['abcdde', 'baccd', 'eeabg']
>>> print(gemstones(arr))
2

edited 1 hour ago

Mathias Ettinger

22.1k32976

asked 1 hour ago

db18

814

add a commentÂ |Â

up vote
4
down vote

favorite

This is the "Gemstones" problem on Hackerrank.

John has collected various rocks. Each rock has various minerals embeded in it. Each type of mineral is designated by a lowercase letter in the range a-z. There may be multiple occurrences of a mineral in a rock. A mineral is called a gemstone if it occurs at least once in each of the rocks in John's collection.
Given a list of minerals embedded in each of John's rocks, display the number of types of gemstones he has in his collection.
For example, the array of mineral composition strings [abc, abc, bc] The minerals b and c appear in each composite, so there are 2 gemstones.

This is my Python code:

def gemstones(arr): 
 for i in range(len(arr)):
 arr[i] = "".join(set(arr[i]))

 long = max(arr, key=len)
 arrlen = len(arr)
 flag,count = 0,0
 for i in long: 
 for j in range(arrlen):
 if i not in arr[j]: flag = 1
 if flag is 0: count += 1
 flag = 0
 return count

Are there ways to improve this code? I feel I'm not using the full functionality of Python.

A test case:

>>> arr = ['abcdde', 'baccd', 'eeabg']
>>> print(gemstones(arr))
2

edited 1 hour ago

Mathias Ettinger

22.1k32976

asked 1 hour ago

db18

814

This is the "Gemstones" problem on Hackerrank.

John has collected various rocks. Each rock has various minerals embeded in it. Each type of mineral is designated by a lowercase letter in the range a-z. There may be multiple occurrences of a mineral in a rock. A mineral is called a gemstone if it occurs at least once in each of the rocks in John's collection.
Given a list of minerals embedded in each of John's rocks, display the number of types of gemstones he has in his collection.
For example, the array of mineral composition strings [abc, abc, bc] The minerals b and c appear in each composite, so there are 2 gemstones.

This is my Python code:

def gemstones(arr): 
 for i in range(len(arr)):
 arr[i] = "".join(set(arr[i]))

 long = max(arr, key=len)
 arrlen = len(arr)
 flag,count = 0,0
 for i in long: 
 for j in range(arrlen):
 if i not in arr[j]: flag = 1
 if flag is 0: count += 1
 flag = 0
 return count

Are there ways to improve this code? I feel I'm not using the full functionality of Python.

A test case:

>>> arr = ['abcdde', 'baccd', 'eeabg']
>>> print(gemstones(arr))
2

python strings programming-challenge

edited 1 hour ago

Mathias Ettinger

22.1k32976

asked 1 hour ago

db18

814

edited 1 hour ago

Mathias Ettinger

22.1k32976

asked 1 hour ago

db18

814

edited 1 hour ago

Mathias Ettinger

22.1k32976

edited 1 hour ago

Mathias Ettinger

22.1k32976

edited 1 hour ago

Mathias Ettinger

22.1k32976

asked 1 hour ago

db18

814

asked 1 hour ago

db18

814

asked 1 hour ago

db18

814

add a commentÂ |Â

2 Answers
2

active

oldest

votes

up vote
3
down vote

You need to stop iterating over indexes and loop like a native. A first rewrite with that in mind would yield:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 for rock in collection:
 if mineral not in rock:
 break
 else:
 count += 1
 return count

This version make use for the for...else construct. But the all() builtin would be more expressive:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 if all(mineral in rock for rock in collection):
 count += 1

 return count

And counting a condition in a for-loop can be more efficient using sum():

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 return sum(
 all(mineral in rock for rock in collection)
 for mineral in biggest_rock
 )

Other than that, you won't have more gemstones than the amount of minerals in the smallest rock, so why doing extra work by using the biggest one to start with?

But you are going back and forth between strings and sets when the problem clearly calls for set intersection repeated on several elements of an array. Luckily, the set.intersection method accept a variable number of arguments. Just make sure to catch any error thrown in case the original collection is empty:

def gemstone(collection):
 rocks = map(set, collection)
 try:
 minerals = next(rocks)
 except StopIteration:
 return 0 # If the collection is empty, there is no gemstones
 return len(minerals.intersection(*rocks))

edited 40 mins ago

answered 1 hour ago

Mathias Ettinger

22.1k32976

# If the collection is empty, there are no gemstones Python... always the naive implementation that gets ripped to shreds by native implementation... I'm not sure there anything that you can do in python that can't be turned into a list of native operations... Oh wait. Same goes for every language... Maybe python just has more than js.
â€“Â FreezePhoenix
16 mins ago

@FreezePhoenix I don't get what you're trying to convey. I could have used next(rocks, set()) to avoid the whole try and achieve the same result, is it somewhat related?
â€“Â Mathias Ettinger
9 mins ago

add a commentÂ |Â

up vote
0
down vote

An alternative way to solve this is to just write down the problem description in Python:

rocks = ['abcdde', 'baccd', 'eeabg']


def is_gemstone(mineral):
 return all(mineral in rock for rock in rocks)


minerals = mineral for rock in rocks for mineral in rock
gemstones = mineral for mineral in minerals if is_gemstone(mineral)

print(len(gemstones))

answered 20 mins ago

folkol

1011

New contributor

add a commentÂ |Â

Your Answer

StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);

);

draft saved

draft discarded

StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f204712%2fcount-number-of-gemstones-in-a-set-of-minerals%23new-answer', 'question_page');

);

Post as a guest

Name

2 Answers
2

active

oldest

votes

2 Answers
2

active

oldest

votes

up vote
3
down vote

You need to stop iterating over indexes and loop like a native. A first rewrite with that in mind would yield:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 for rock in collection:
 if mineral not in rock:
 break
 else:
 count += 1
 return count

This version make use for the for...else construct. But the all() builtin would be more expressive:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 if all(mineral in rock for rock in collection):
 count += 1

 return count

And counting a condition in a for-loop can be more efficient using sum():

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 return sum(
 all(mineral in rock for rock in collection)
 for mineral in biggest_rock
 )

Other than that, you won't have more gemstones than the amount of minerals in the smallest rock, so why doing extra work by using the biggest one to start with?

def gemstone(collection):
 rocks = map(set, collection)
 try:
 minerals = next(rocks)
 except StopIteration:
 return 0 # If the collection is empty, there is no gemstones
 return len(minerals.intersection(*rocks))

edited 40 mins ago

answered 1 hour ago

Mathias Ettinger

22.1k32976

# If the collection is empty, there are no gemstones Python... always the naive implementation that gets ripped to shreds by native implementation... I'm not sure there anything that you can do in python that can't be turned into a list of native operations... Oh wait. Same goes for every language... Maybe python just has more than js.
â€“Â FreezePhoenix
16 mins ago

@FreezePhoenix I don't get what you're trying to convey. I could have used next(rocks, set()) to avoid the whole try and achieve the same result, is it somewhat related?
â€“Â Mathias Ettinger
9 mins ago

add a commentÂ |Â

up vote
3
down vote

You need to stop iterating over indexes and loop like a native. A first rewrite with that in mind would yield:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 for rock in collection:
 if mineral not in rock:
 break
 else:
 count += 1
 return count

This version make use for the for...else construct. But the all() builtin would be more expressive:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 if all(mineral in rock for rock in collection):
 count += 1

 return count

And counting a condition in a for-loop can be more efficient using sum():

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 return sum(
 all(mineral in rock for rock in collection)
 for mineral in biggest_rock
 )

Other than that, you won't have more gemstones than the amount of minerals in the smallest rock, so why doing extra work by using the biggest one to start with?

def gemstone(collection):
 rocks = map(set, collection)
 try:
 minerals = next(rocks)
 except StopIteration:
 return 0 # If the collection is empty, there is no gemstones
 return len(minerals.intersection(*rocks))

edited 40 mins ago

answered 1 hour ago

Mathias Ettinger

22.1k32976

# If the collection is empty, there are no gemstones Python... always the naive implementation that gets ripped to shreds by native implementation... I'm not sure there anything that you can do in python that can't be turned into a list of native operations... Oh wait. Same goes for every language... Maybe python just has more than js.
â€“Â FreezePhoenix
16 mins ago

@FreezePhoenix I don't get what you're trying to convey. I could have used next(rocks, set()) to avoid the whole try and achieve the same result, is it somewhat related?
â€“Â Mathias Ettinger
9 mins ago

add a commentÂ |Â

up vote
3
down vote

You need to stop iterating over indexes and loop like a native. A first rewrite with that in mind would yield:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 for rock in collection:
 if mineral not in rock:
 break
 else:
 count += 1
 return count

This version make use for the for...else construct. But the all() builtin would be more expressive:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 if all(mineral in rock for rock in collection):
 count += 1

 return count

And counting a condition in a for-loop can be more efficient using sum():

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 return sum(
 all(mineral in rock for rock in collection)
 for mineral in biggest_rock
 )

Other than that, you won't have more gemstones than the amount of minerals in the smallest rock, so why doing extra work by using the biggest one to start with?

def gemstone(collection):
 rocks = map(set, collection)
 try:
 minerals = next(rocks)
 except StopIteration:
 return 0 # If the collection is empty, there is no gemstones
 return len(minerals.intersection(*rocks))

edited 40 mins ago

answered 1 hour ago

Mathias Ettinger

22.1k32976

You need to stop iterating over indexes and loop like a native. A first rewrite with that in mind would yield:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 for rock in collection:
 if mineral not in rock:
 break
 else:
 count += 1
 return count

This version make use for the for...else construct. But the all() builtin would be more expressive:

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 count = 0
 for mineral in biggest_rock:
 if all(mineral in rock for rock in collection):
 count += 1

 return count

And counting a condition in a for-loop can be more efficient using sum():

def gemstones(collection):
 collection = [''.join(set(rock)) for rock in collection]
 biggest_rock = max(collection, key=len)

 return sum(
 all(mineral in rock for rock in collection)
 for mineral in biggest_rock
 )

Other than that, you won't have more gemstones than the amount of minerals in the smallest rock, so why doing extra work by using the biggest one to start with?

def gemstone(collection):
 rocks = map(set, collection)
 try:
 minerals = next(rocks)
 except StopIteration:
 return 0 # If the collection is empty, there is no gemstones
 return len(minerals.intersection(*rocks))

edited 40 mins ago

answered 1 hour ago

Mathias Ettinger

22.1k32976

edited 40 mins ago

answered 1 hour ago

Mathias Ettinger

22.1k32976

answered 1 hour ago

Mathias Ettinger

22.1k32976

answered 1 hour ago

Mathias Ettinger

22.1k32976

# If the collection is empty, there are no gemstones Python... always the naive implementation that gets ripped to shreds by native implementation... I'm not sure there anything that you can do in python that can't be turned into a list of native operations... Oh wait. Same goes for every language... Maybe python just has more than js.
â€“Â FreezePhoenix
16 mins ago

@FreezePhoenix I don't get what you're trying to convey. I could have used next(rocks, set()) to avoid the whole try and achieve the same result, is it somewhat related?
â€“Â Mathias Ettinger
9 mins ago

add a commentÂ |Â

# If the collection is empty, there are no gemstones Python... always the naive implementation that gets ripped to shreds by native implementation... I'm not sure there anything that you can do in python that can't be turned into a list of native operations... Oh wait. Same goes for every language... Maybe python just has more than js.
â€“Â FreezePhoenix
16 mins ago

@FreezePhoenix I don't get what you're trying to convey. I could have used next(rocks, set()) to avoid the whole try and achieve the same result, is it somewhat related?
â€“Â Mathias Ettinger
9 mins ago

# If the collection is empty, there are no gemstones Python... always the naive implementation that gets ripped to shreds by native implementation... I'm not sure there anything that you can do in python that can't be turned into a list of native operations... Oh wait. Same goes for every language... Maybe python just has more than js.
â€“Â FreezePhoenix
16 mins ago

@FreezePhoenix I don't get what you're trying to convey. I could have used next(rocks, set()) to avoid the whole try and achieve the same result, is it somewhat related?
â€“Â Mathias Ettinger
9 mins ago

add a commentÂ |Â

up vote
0
down vote

An alternative way to solve this is to just write down the problem description in Python:

rocks = ['abcdde', 'baccd', 'eeabg']


def is_gemstone(mineral):
 return all(mineral in rock for rock in rocks)


minerals = mineral for rock in rocks for mineral in rock
gemstones = mineral for mineral in minerals if is_gemstone(mineral)

print(len(gemstones))

answered 20 mins ago

folkol

1011

New contributor

add a commentÂ |Â

up vote
0
down vote

An alternative way to solve this is to just write down the problem description in Python:

rocks = ['abcdde', 'baccd', 'eeabg']


def is_gemstone(mineral):
 return all(mineral in rock for rock in rocks)


minerals = mineral for rock in rocks for mineral in rock
gemstones = mineral for mineral in minerals if is_gemstone(mineral)

print(len(gemstones))

answered 20 mins ago

folkol

1011

New contributor

add a commentÂ |Â

up vote
0
down vote

An alternative way to solve this is to just write down the problem description in Python:

rocks = ['abcdde', 'baccd', 'eeabg']


def is_gemstone(mineral):
 return all(mineral in rock for rock in rocks)


minerals = mineral for rock in rocks for mineral in rock
gemstones = mineral for mineral in minerals if is_gemstone(mineral)

print(len(gemstones))

answered 20 mins ago

folkol

1011

New contributor

An alternative way to solve this is to just write down the problem description in Python:

rocks = ['abcdde', 'baccd', 'eeabg']


def is_gemstone(mineral):
 return all(mineral in rock for rock in rocks)


minerals = mineral for rock in rocks for mineral in rock
gemstones = mineral for mineral in minerals if is_gemstone(mineral)

print(len(gemstones))

answered 20 mins ago

folkol

1011

New contributor

answered 20 mins ago

folkol

1011

New contributor

answered 20 mins ago

folkol

1011

answered 20 mins ago

folkol

1011

New contributor

folkol is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.

add a commentÂ |Â

draft saved

draft discarded

draft saved

draft discarded

Post as a guest

Name

Search This Blog

Iyfjky