Skip to content
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

My solution to problems 1, 2 and 3 #50

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

scay0x
Copy link

@scay0x scay0x commented Nov 22, 2020

Some mentions for problem number 3:

  • I resolved the problem in multiple ways, because I was not sure about each proposed solution.
  • However I still proposed a solution with nested fors, although the problem requirement clearly specified to not.

Copy link
Member

@alexandru-calinoiu alexandru-calinoiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a look at the problems, apprecite the fact that you've done them in kotlin, made a couple of commets take a look.

The main problem is that you did not do any unit tests to prove the functionality of any of the solutions.

@@ -0,0 +1,11 @@
fun main() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation is straight forward

The idea was that you get an array of n elements as an input, you don't have to generate the input.

Comment on lines +5 to +9
for(i in arr) {
if (i % 2 == 0) {
sum += arr[i]
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There a couple of usefull functions for arrays in kotlin scout the docs maybe you can find a way to make this more idiomatic.

Comment on lines +6 to +19
for(valoare_i in arr) {
if(valoare_i > max1) {
max1 = valoare_i
}else {
max1 = max1
}
}
for(valoare_j in arr) {
if (valoare_j > max2 && valoare_j != max1) {
max2 = valoare_j
} else {
max2 = max2
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you think of a way to do this in one loop?

Also what about some edge cases? empty array, one item array ....

for(i in a.indices) {
if(i == 0) {
distincte += (a[i].toString() + " ")
} else if(distincte.contains(" " + a[i])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contains most likely uses a loop in it's implementation, so you are still using a loop inside a loop, there is a clear way to solve this problem.

@deniscoman
Copy link

Beside what @alexandru-calinoiu said, you have some naming issues that makes your code harder to read.

  1. It's better to write all your code in English so anyone can understand what are you trying to do.
  2. Here you can find some naming conventions that will help you, and anyone else who read your code, to understand what your code does. This will be helpful in the feature no matter what programming language you will use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants