Skip to content

Finished exercise 7 #4

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Finished exercise 7 #4

wants to merge 21 commits into from

Conversation

bojanludajic
Copy link

No description provided.

Comment on lines +21 to +22
val prvi = this.sorted().get(this.lastIndex)
val drugi = this.sorted().get(this.lastIndex-1)
Copy link
Owner

Choose a reason for hiding this comment

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

There is no need to sort the array twice. Please name the variables in English, as that is a widely accepted convention.

@@ -20,7 +20,9 @@ import org.jetbrains.exercise2.task3.findPairWithBiggestDifference
*/

internal fun List<Int>.findHighestSumPairFunctional(): Pair<Int, Int> {
TODO("Implement me!!")
return Pair(
this.sorted().get(this.lastIndex), this.sorted().get(this.lastIndex-1)
Copy link
Owner

Choose a reason for hiding this comment

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

This is not a functional approach :)

The idea of the functional approach is that you compose function calls that operate on data to get the desired result.

Functional approach would be following:

Suggested change
this.sorted().get(this.lastIndex), this.sorted().get(this.lastIndex-1)
return this.sorted().let { sortedList -> Pair(this.lastIndex, this.lastIndex-1) }

In this case, we are using the result of the sorted function invocation, and we are providing it as an input of the let function invocation. Hence, we are composing function calls to get the result.


return resultPair!!
return Pair(
this.sorted().get(this.size-1), this.sorted().get(0)
Copy link
Owner

Choose a reason for hiding this comment

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

The solution is correct, but it is written in Java-style code. Please consider my suggestion above to implement it in a Kotlin, functional style. :)

Also, this.size - 1 has a Kotlin equivalent in this.lastIndex, and get(0) has an equivalent in a first() function.

)


// var resultPair: Pair<Int, Int>? = null
Copy link
Owner

Choose a reason for hiding this comment

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

Please clean up the code before pushing the changes.

}

internal fun List<Country>.findLanguageSpokenInMostCountries(): String {
TODO("Implement me!!!")
return countries.flatMap { it.languages }
Copy link
Owner

Choose a reason for hiding this comment

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

Very nice solution!

@@ -49,11 +49,15 @@ internal data class Address(
*/

internal fun user(initUser: User.() -> Unit): User {
TODO("Implement me!!!")
val user = User()
return user.apply(initUser)
Copy link
Owner

Choose a reason for hiding this comment

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

nit: this can be a one-line expression :)

internal fun User.address(initAddress: Address.() -> Unit): User {
TODO("Implement me!!!")
internal fun User.address (initAddress: Address.() -> Unit): User {
val adr = Address()
Copy link
Owner

Choose a reason for hiding this comment

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

That's it! Why not using apply scoped function here as well?

@vuksa
Copy link
Owner

vuksa commented Mar 26, 2024

Please try going one more time through functional programming, and what the way is to work with collections in a Kotlin style. The task about countries showed that you can do it, but your solutions for finding pairs need to have a functional approach to solving the problem as well.

One side note: when you write code, please write code in English(name methods and variables in English). It is a widely spread convention, and once you get into the industry, you'll work in an international environment, which will require you to write code in such a way. Also, please keep your code clean, and do not push commented-out code. :)

@vuksa vuksa force-pushed the main branch 2 times, most recently from 9db668d to 189c667 Compare March 27, 2024 16:39
@vuksa vuksa added the exercise3 label Apr 6, 2024
@@ -26,9 +27,32 @@ package exercise3.task1


internal fun isExpressionBalanced(expression: String): Boolean {
TODO("Implement me!!!")
val chars = Stack<Char>()
Copy link
Owner

Choose a reason for hiding this comment

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

The solution is correct. 👏🏻
Please now try solving it in the functional way. :)

var counter = 0
var balanced = true

for(i in this.indices) {
Copy link
Owner

Choose a reason for hiding this comment

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

You could use for ((index, char) in this.withIndex()) to make for loop more readable

val opening = "({["
val closing = ")}]"
val clusterList = ArrayList<String>()
val emptyList = ArrayList<String>()
Copy link
Owner

Choose a reason for hiding this comment

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

This is only used as a return value at the end of the function.
It's better to move it's declaration closer to the places where it's used.

TODO("Implement me!!!")
val opening = "({["
val closing = ")}]"
val clusterList = ArrayList<String>()
Copy link
Owner

Choose a reason for hiding this comment

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

nit: don't forget that we have mutableListOf top-level function to create list

@@ -24,7 +24,45 @@ package exercise3.task2
*/

internal fun String.splitToBracketsClusters(): List<String> {
TODO("Implement me!!!")
val opening = "({["
Copy link
Owner

Choose a reason for hiding this comment

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

opening and closing variables aren't used

Copy link
Owner

Choose a reason for hiding this comment

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

The solution is correct! It's written in Java imperative way so it's a bit hard to follow the execution flow, and it took me some to understand what it does.

@@ -29,7 +29,24 @@ package exercise3.task3
*/

internal fun isSherlockValid(s: String): String {
TODO("Implement me!!!")
val charCounts = s.groupBy { it }.values.map { it.size }
Copy link
Owner

Choose a reason for hiding this comment

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

The same can be achieved with s.groupingBy { it }.eachCount()

TODO("Implement me!!!")
val charCounts = s.groupBy { it }.values.map { it.size }

if (charCounts.toSet().size == 1) {
Copy link
Owner

Choose a reason for hiding this comment

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

charCounts.toSet() could have be done in the line above where we define charCounts so that we avoid using toSet multiple times.

if (charCounts.toSet().size == 2) {
val minFreq = charCounts.minOrNull() ?: return "NO"
val maxFreq = charCounts.maxOrNull() ?: return "NO"
val minFreqCounter = charCounts.count { it == minFreq }
Copy link
Owner

Choose a reason for hiding this comment

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

If we know that we have two different numbers and get their min and max values, why is it necessary to use a counter here?
Isn't it enough to check if both values are the same, or that maxFreq - minFreq is equal to 1?

@@ -10,29 +10,44 @@ package exercise3.task4
* Find all the drivers who performed no trips.
*/
internal fun TaxiPark.findFakeDrivers(): Set<Driver> {
TODO("Implement me!!!")
return allDrivers.minus(this.trips.map { it.driver }.toSet())
Copy link
Owner

Choose a reason for hiding this comment

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

Well done! 👏🏻

}

/**
* Subtask 4:
* Find the passengers who had a discount for the majority of their trips.
*/
internal fun TaxiPark.findSmartPassengers(): Set<Passenger> {
TODO("Implement me!!!")
return allPassengers.filter { passenger ->
Copy link
Owner

Choose a reason for hiding this comment

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

Well done! 👏🏻

Copy link
Owner

@vuksa vuksa left a comment

Choose a reason for hiding this comment

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

Task4 is really well done! It seems to me that in some solutions you still think in "Java" way but I don't doubt that until end of course you'll adopt the Kotlin approach to the full extend.

@vuksa vuksa added the reviewed label Apr 20, 2024
@bojanludajic bojanludajic changed the title Finished exercise 2 Finished exercise 6 Apr 29, 2024
vuksa and others added 7 commits April 29, 2024 18:16
TO FIX:
-MAKE TEAM NAMES AND DATA JUST VALUES, WITHOUT NAMES OF EACH VALUE;
-FIX PARSING LOGIC FOR EACH FIXTURE, 72200 MATCHES SHOWN INSTEAD OF 38 FOR EACH TEAM
-IMPLEMENT numOfGoalsTeamScoredAagainst, numOfGoalsTeamConcededAgainst and
displayLeagueTableAtFixture functions
FIXED : PARSING FIXTURES, FORMATTED TABLE
@bojanludajic bojanludajic changed the title Finished exercise 6 Finished exercise 7 May 5, 2024
@vuksa vuksa force-pushed the main branch 3 times, most recently from 6106c56 to 8296ece Compare May 20, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants