I trying to convert this C code for calculating distance from RSSI to Swift code. I try to do it by myself, but considering I'm beginner, I need help in how to do it.
Here is C code:
#define QUEUE_SIZE 16
#define INCREASE_INDEX(x) ({x++; if(x >= QUEUE_SIZE) x = 0;})
int rssi_array[QUEUE_SIZE] = {0};
int sort_rssi[QUEUE_SIZE] = {0};
int rssi_index = 0;
static double getDistance(double rssi, int txPower) {
/*
* RSSI = TxPower - 10 * n * lg(d)
* n = 2 (in free space)
*
* d = 10 ^ ((TxPower - RSSI) / (10 * n))
*/
if (rssi == 0) {
return -1.0; // if we cannot determine accuracy, return -1.
}
return pow(10, ((double) txPower - rssi) / (10 * 2));
}
static double calculateAccuracy(double rssi, int txPower) {
if (rssi == 0) {
return -1.0; // if we cannot determine accuracy, return -1.
}
double ratio = rssi * 1.0 / txPower;
if (ratio < 1.0) {
return pow(ratio, 10);
} else {
double accuracy = (0.89976) * pow(ratio, 7.7095) + 0.111;
return accuracy;
}
}
int cmpfunc (const void * a, const void * b) {
return ( *(int*)a - *(int*)b );
}
static double calculate_average() {
double average = 0;
int i = 0;
int drop = 3;
memcpy(sort_rssi, rssi_array, QUEUE_SIZE * sizeof(int));
qsort(sort_rssi, QUEUE_SIZE, sizeof(int), cmpfunc);
for (i = 0; i < QUEUE_SIZE - drop; ++i) {
if(sort_rssi[i + drop] == 0) break;
average += sort_rssi[i];
}
return average / i;
}
// For adding new rssi we can use this code:
rssi_array[rssi_index] = rssi;
INCREASE_INDEX(rssi_index);
double mean_rssi = calculate_average();
Here is what I managed to do, but I'm not sure if it's good. Swift code:
let queueSize = 16
func increaseIndex(_ x: Int) -> Int {
var x = x + 1
if x >= queueSize {
x = 0
}
return x
}
var rssiArray = [0]
var sortRssi = [0]
var rssiIndex = 0
private func getDistance(rssi: Double, txPower: Int) -> Double {
if rssi == 0 {
return -1.0 // if we cannot determine accuracy, return -1.
}
return pow(10, (Double(txPower) - rssi) / (10 * 2))
}
private func calculateAccuracy(rssi: Double, txPower: Int) -> Double {
if rssi == 0 {
return -1.0 // if we cannot determine accuracy, return -1.
}
let ratio = rssi * 1.0 / Double(txPower)
if ratio < 1.0 {
return pow(ratio, 10)
} else {
let accuracy = (0.89976) * pow(ratio, 7.7095) + 0.111
return accuracy
}
}
// This part is probably for comparing results from two functions,
// but I don't get it from where and what
func compareFunction(_ a: UnsafeRawPointer?, _ b: UnsafeRawPointer?) -> Int {
return Int(a ?? 0) - Int(b ?? 0)
}
private func calculateAverage() -> Double {
var average: Double = 0
var i = 0
let drop = 3
memcpy(sortRssi, rssiArray, queueSize * MemoryLayout<Int>.size)
qsort(sortRssi, queueSize, MemoryLayout<Int>.size, cmpfunc)
for i in 0..<queueSize - drop {
if sortRssi[i + drop] == 0 {
break
}
average += Double(sortRssi[i])
}
return average / Double(i)
}
I know this will be very useful for others. Code help will be much appreciated.
Thank you all in advance!
2 Answers 2
A few suggestions:
- Don't have everything be part of a class. In Swift, you can have free functions, just like in C. Remove the
class AverageRSSI: NSObject { ... }
. - Keep casing consistent. You have
calculateAccuracy
andcalculate_average
. The preferred casing in Swift iscamelCase
. - It's customary in Swift not to capitalize constant and function names. Everything that's not a class name should be
camelCase
. - Have more meaningful names.
calculateAccuracy
is good,cmpfunc
is not.rssi_array
could also berssi_values
or simplyrssi
. - Make both the function definition and the function call as clear as possible. For example, when you read
calculateAccuracy(_ rssi: Double, _ txPower: Int)
, it's clear that the first parameter isrssi
and the second istxPower
, but when you call it ascalculateAccuracy(1.0, 2)
, it's no longer clear. Keep the parameter names withcalculateAccuracy(rssi: Double, txPower: Int)
and call it ascalculateAccuracy(rssi: 1.0, txPower: 2)
. getDistance
andcalculateAccuracy
return the magic value-1
in case of error. In Swift it's better to throw an error detailing what the problem was, instead of returning an invalid value.- Since
rssi_array
andsort_rssi
are arrays, you don't need to usememcpy
to copy the values of one to another, you can simply assign the values withsort_rssi = rssi_array
. - In Swift, you don't need to use the C
qsort()
function, since anyMutableCollection
has asort()
method. - Since
sort_rssi
is only used inside a function, it doesn't need to be file global, it can be a local variable.
You could replace
memcpy(sort_rssi, rssi_array, QUEUE_SIZE * sizeof(int));
qsort(sort_rssi, QUEUE_SIZE, sizeof(int), cmpfunc);
with:
var sortedRssi = rssiArray
sortedRssi.sort(by: >)
See:
- Swift API Design Guidelines for a list of Swift coding guidelines.
- Collection Types for details about collections, including arrays.
- Error Handling for details about how to create, throw and catch errors.
-
\$\begingroup\$ Thanks @rid I will correct the code according to your suggestions and update the question. How to deal issue with memcpy and qsort? What is the solution in swift? \$\endgroup\$Win Fan– Win Fan2020年05月16日 10:10:28 +00:00Commented May 16, 2020 at 10:10
-
\$\begingroup\$ @AppleFan, updated answer with some more suggestions, including some related to
memcpy()
andqsort()
. \$\endgroup\$rid– rid2020年05月16日 10:27:27 +00:00Commented May 16, 2020 at 10:27 -
\$\begingroup\$ @AppleFan, updated answer to mention
sort()
that's part of any mutable collection, such as arrays, and added a usage example. \$\endgroup\$rid– rid2020年05月16日 10:38:57 +00:00Commented May 16, 2020 at 10:38 -
\$\begingroup\$ I see that in C using
memcpy
,QUEUE_SIZE
is used. If I replacememcpy
withvar sortedRssi = rssiArray
, how comequeueSize
is not used? \$\endgroup\$Win Fan– Win Fan2020年05月16日 11:37:21 +00:00Commented May 16, 2020 at 11:37 -
1\$\begingroup\$ @AppleFan, in C, an "array" is just a pointer to a place in memory that contains values one after the other. It doesn't have the concept of size or length.
memcpy()
simply copies bytes from one part of the memory to another, and you need to tell it from where, to where, and how many bytes. In Swift, arrays are objects that contain metadata such as length, so the Swift array has all the knowledge it needs to make a copy of itself to another array. \$\endgroup\$rid– rid2020年05月16日 13:18:24 +00:00Commented May 16, 2020 at 13:18
Since this is a good chunk of my day job, I've got a bit of feedback for you 🙂
Just keep in mind that Swift is a very flexible language! Your translation works, so you can use as much or as little of that as you want, depending on your or your team's preferences.
Embrace Immutability
C was designed to be sugar atop Assembly, which emulates a Turing machine, so it's very good at mutation. Swift was designed to be algorithmic, declarative, and functional, so it's very good at copying. Because of this, Swift code will work and optimize better if you embrace immutability.
For example, you already did a little bit of this in increaseIndex(_:)
by making it take an input and return an output, rather than mutating some global variable:
func increaseIndex(_ x: Int) -> Int {
var x = x + 1
if x >= queueSize {
x = 0
}
return x
}
In C, this is good because it works with C's talents of mutating memory. However, this comes at the cost of clarity: If x
is a var
, where is it mutated? What is its value when it's returned and when might that change? If I want to write more code in this function, how, where, and why should I mutate x
? I have to think like a machine, stepping through the code in my head and keeping track of state, just to understand what will be returned.
Compare that to this version:
func increaseIndex(_ x: Int) -> Int {
let x = x + 1
if x >= queueSize {
return 0
}
return x
}
Here, we know that x
has the same value throughout the function, and will never change, since it's declared as a let
constant. We can see at the declaration site that it's always the input plus one. We can also see that there's a branch with a special case, and the only code in there is return 0
. So we know that this function can only ever return its input plus one, or zero.
We can do better, though, if we...
Embrace language constructs
Taking the same example above, there are a few more approaches that Swift allows. Let's look at this one:
func increaseIndex(_ x: Int) -> Int {
let x = x + 1
guard x < queueSize else {
return 0
}
return x
}
The guard
statement! It works kind of like other languages' unless
, except it must always leave scope (in this case, return
. In loops, continue
and break
can also work, etc.). This gives us the peace-of-mind that, since we see a guard
, absolutely nothing after it will be executed if its condition is false. We also know that maintainers can't mess this up solely by changing the body of the guard
, since the compiler enforces that the body must return from this function no matter what.
This is another approach that's available to us:
func increaseIndex(_ x: Int) -> Int {
let x = x + 1
return x < queueSize
? x
: 0
}
Ternary operators aren't everyone's cup-of-tea, so I understand if you don't like this approach. Personally, I find it much better because you can see this function has two lines: it declares a constant and then returns. No possibility of it doing anything else, and it's clear that it either returns that constant or zero, depending on the condition we see.
Here's another place where Swift can help things be clearer:
private func getDistance(rssi: Double, txPower: Int) -> Double {
if rssi == 0 {
return -1.0 // if we cannot determine accuracy, return -1.
}
return pow(10, (Double(txPower) - rssi) / (10 * 2))
}
Your comment there is necessary because you're using a magic number. This is unnecessary in Swift, since enum
s are very cheap and lightweight, and have associated values. The Swift compiler's optimizer will take care of erasing the enum's cases where possible, too, so you don't have to worry about (un)boxing. So, this function can be made clearer with an Optional
return!
private func getDistance(rssi: Double, txPower: Int) -> Double? {
if rssi == 0 {
return nil
}
return pow(10, (Double(txPower) - rssi) / (10 * 2))
}
And, if you're OK using ternary operators, it can further be simplified to this:
private func getDistance(rssi: Double, txPower: Int) -> Double? {
return rssi == 0
? nil
: pow(10, (Double(txPower) - rssi) / 20)
}
Swift separates literals from types
In C and its descendents, each primitive has its own literal, and strings exist too. In Swift, literals can represent anything, and default to primitives if no type is specified. In fact, you take advantage of this in a few places already!
This means that you don't have to write 1.0
or similar where it's clear a Double
is expected; you can simply write 1
. This makes the language more human. For me, it makes it easier to read too.
Lots to learn in calculateAverage()
Last up, let's look at calculateAverage()
. At first glance, I'm not sure what's intended here, nor how it works. There's a lot of internal state and low-level operations, and a recreation of a C-style for-loop. These are big code smells for me, telling me there's stuff to be done here:
var rssiArray = [0]
var sortRssi = [0]
private func calculateAverage() -> Double {
var average: Double = 0
var i = 0
let drop = 3
memcpy(sortRssi, rssiArray, queueSize * MemoryLayout<Int>.size)
qsort(sortRssi, queueSize, MemoryLayout<Int>.size, cmpfunc)
for i in 0..<queueSize - drop {
if sortRssi[i + drop] == 0 {
break
}
average += Double(sortRssi[i])
}
return average / Double(i)
}
First, as rid said, the sorting can be simplified greatly by taking advantage of Swift's built-in memory management and standard library functions:
memcpy(sortRssi, rssiArray, queueSize * MemoryLayout<Int>.size)
qsort(sortRssi, queueSize, MemoryLayout<Int>.size, cmpfunc)
// becomes:
sortRssi = rssiArray.sorted(by: >)
This also means that compareFunction
is unnecessary!
Next, I see a constant drop = 3
. It appears to me you're using this to drop the first 3 values from the array, which can be replaced with the standard library function dropFirst(_:)
. This returns a lazily-evaluated sequence, so we'll also have to create an array from that:
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
for i in 0..<sortRssi.count {
if sortRssi[i] == 0 {
break
}
average += Double(sortRssi[i])
}
I also notice that you're keeping track of an i
variable here, and returning it at the end. This is also unnecessary since now its value will be the same as sortRssi.count
, and Swift only allows for-each loops anyway:
private func calculateAverage() -> Double {
var average: Double = 0
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
for item in sortRssi {
if item == 0 {
break
}
average += Double(item)
}
return average / Double(sortRssi.count)
}
Looking better already! We can further simplify this by taking advantage of the Swift higher-order function reduce
. As the name implies, this reduces a collection of values down to a single value, which is exactly what you want in an averaging function: to reduce an array of numbers to a single number representing the average:
private func calculateAverage() -> Double {
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
let total: Double = sortRssi.reduce(into: 0) { average, item in
if item == 0 {
break // !! Doesn't compile!
}
average += Double(item)
}
return total / Double(sortRssi.count)
}
Oh my! It seems that reduce
can't work here because of this special condition of your program! Not to worry, we can take care of that with another standard library function, prefix(while:)
. This lets us take only the first however-many items in the array, so long as they all match a certain condition, just like you're doing here:
let total: Double = sortRssi
.prefix(while: { 0ドル != 0 })
.reduce(into: 0) { average, item in
average += Double(item)
}
As a C dev, I'm sure this is setting off your big-O alarm, since it loops twice! Not to worry, there's this magical little gem: .lazy
. It allows any collection to be lazily-evaluated, so the loop only runs once:
let total: Double = sortRssi
.lazy
.prefix(while: { 0ドル != 0 })
.reduce(into: 0) { average, item in
average += Double(item)
}
And now since total
is created on one line and used once on the next, we don't have to dedicate a line to declaring it, so we can remove it altogether and get this:
private func calculateAverage() -> Double {
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
return sortRssi
.lazy
.prefix(while: { 0ドル != 0 })
.reduce(into: Double(0)) { average, item in
average += Double(item)
}
/ Double(sortRssi.count)
}
And since we're already in lazy-land, we can restructure this a little more if we want with another standard library function, map
:
private func calculateAverage() -> Double {
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
return sortRssi
.lazy
.prefix(while: { 0ドル != 0 })
.map(Double.init)
.reduce(into: 0) { average, item in
average += item
}
/ Double(sortRssi.count)
}
And now since the body of the reduce
only has one function call in it (+=
), we can remove the body and replace it with a reference to that function:
private func calculateAverage() -> Double {
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
return sortRssi
.lazy
.prefix(while: { 0ドル != 0 })
.map(Double.init)
.reduce(into: 0, +=)
/ Double(sortRssi.count)
}
This looks much better to me. It is very Swifty! There's no internal state, it's all standard Swift functions,
If you want, though, it can be made to fit the philosophies of Swift even better by removing it from state and placing it in a type-extension, to make it more reusable in other places:
private func calculateAverage() -> Double {
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
return sortRssi.averaged(while: { 0ドル != 0 })
}
extension Collection where Element: BinaryInteger {
func averaged(while allowedPrefixPredicate: (Element) -> Bool) -> Double {
lazy
.prefix(while: allowedPrefixPredicate)
.map(Double.init)
.reduce(into: 0, +=)
/ Double(count)
}
}
There it is. Now, the work we put into creating this averaging function can be reused for any collection containing integers. We use it here on an array of Int
s, but later if you want similar behavior, you can use it on a set of UInt8
s, etc!
Putting it all together
Embracing these patterns makes this more reliable. Let's see how it looks after we put all this together in your program:
let queueSize = 16
var rssiArray = [0]
var sortRssi = [0]
var rssiIndex = 0
func increaseIndex(_ x: Int) -> Int {
let x = x + 1
return x < queueSize
? x
: 0
}
private func getDistance(rssi: Double, txPower: Int) -> Double? {
return rssi == 0
? nil
: pow(10, (Double(txPower) - rssi) / 20)
}
private func calculateAccuracy(rssi: Double, txPower: Int) -> Double? {
guard rssi != 0 else {
return nil
}
let ratio = rssi * (1 / Double(txPower))
return ratio < 1
? pow(ratio, 10)
: (0.89976) * pow(ratio, 7.7095) + 0.111
}
private func calculateAverage() -> Double {
sortRssi = Array(rssiArray.sorted(by: >).dropFirst(3))
return sortRssi.averaged(while: { 0ドル != 0 })
}
extension Collection where Element: BinaryInteger {
func averaged(while allowedPrefixPredicate: (Element) -> Bool) -> Double {
lazy
.prefix(while: allowedPrefixPredicate)
.map(Double.init)
.reduce(into: 0, +=)
/ Double(count)
}
}
This looks good to me. It maintains the original functionality, and embraces Swift as much as it can. I hope it helps!
Compare before and after
After adjusting for whitespace and comment lines, and tweaking names a bit to be more comparable, here's a diff comparing the original C code to my Swift translation:
Double
can be represented with1
, omitting the.0
when it's clear that aDouble
goes there (comparing, passing to a function, etc.). \$\endgroup\$