2

I've been trying out Swift/SwiftUI for the first time, so I decided to make a small Hacker News client. I seem to be able to get the list of ids of the top stories back fine, but once the dispatchGroup gets involved, nothing works. What am I doing wrong?

Data.swift

import SwiftUI

struct HNStory: Codable, Identifiable {
    var id: UInt
    var title: String
    var score: UInt
}

class Fetch {
    func getStory(id: Int, completion: @escaping (HNStory) -> ()) {
        let url = URL(string: "https://hacker-news.firebaseio.com/v0/item/\(id).json")
        
        URLSession.shared.dataTask(with: url!) { (data, _, _) in
            let story = try!JSONDecoder().decode(HNStory.self, from: data!)
            print(id, story)
            
            DispatchQueue.main.async {
                completion(story)
            }
        }
    }
    
    func getStories(completion: @escaping ([HNStory]) -> ()) {
        let url = URL(string: "https://hacker-news.firebaseio.com/v0/topstories.json")
        var stories: [HNStory] = []
        print("here")
        
        URLSession.shared.dataTask(with: url!) { (data, _, _) in
            var ids = try!JSONDecoder().decode([Int].self, from: data!)
            
            ids = Array(ids[0...10])
            print(ids)
            
            let dispatchGroup = DispatchGroup()
            
            for id in ids {
                dispatchGroup.enter()
                self.getStory(id: id) { (story) in
                    stories.append(story)
                    dispatchGroup.leave()
                }
            }
            
            dispatchGroup.notify(queue: .main) {
                print("Completed work")
                DispatchQueue.main.async {
                    completion(stories)
                }
            }
        }.resume()
    }
}

ContentView.swift (probably doesn't matter, but just in case)

import SwiftUI

struct ContentView: View {
    @State private var stories: [HNStory] = []

    var body: some View {
        Text("Hacker News").font(.headline)
        List(stories) { story in
            VStack {
                Text(story.title)
                Text(story.score.description)
            }
        }.onAppear{
            Fetch().getStories { (stories) in
                self.stories = stories
            }
        }
    }
}

struct ContentView_Previews: PreviewProvider {
    static var previews: some View {
        ContentView()
    }
}

3 Answers3

2

One major problem is this line:

Fetch().getStories...

Networking takes time. You make a Fetch instance and immediately let it destroy itself. Thus it does not survive long enough to do any networking! You need to configure a singleton that persists.

Another problem, as OOPer points out in a comment, is that your getStory creates a data task but never tells it to resume — so that method is doing no networking at all, even if it did have time to do so.

matt
  • 515,959
  • 87
  • 875
  • 1,141
1

FWIW, with Swift UI, I would suggest you consider using Combine publishers for your network requests.

So, the publisher to get a single story:

func storyUrl(for id: Int) -> URL {
    URL(string: "https://hacker-news.firebaseio.com/v0/item/\(id).json")!
}

func hackerNewsStoryPublisher(for identifier: Int) -> AnyPublisher<HNStory, Error> {
    URLSession.shared.dataTaskPublisher(for: storyUrl(for: identifier))
        .map(\.data)
        .decode(type: HNStory.self, decoder: decoder)
        .eraseToAnyPublisher()
}

And a publisher for a sequence of the above:

func hackerNewsIdsPublisher(for ids: [Int]) -> AnyPublisher<HNStory, Error> {
    Publishers.Sequence(sequence: ids.map { hackerNewsStoryPublisher(for: $0) })
        .flatMap(maxPublishers: .max(4)) { $0 }
        .eraseToAnyPublisher()
}

Note, the above constrains it to four at a time, enjoying the performance gains of concurrency, but limiting it so you do not risk having latter requests time out:

network timeline

Anyway, here is the first fetch of the array of ids and then launching the above:

let mainUrl = URL(string: "https://hacker-news.firebaseio.com/v0/topstories.json")!

func hackerNewsPublisher() -> AnyPublisher<HNStory, Error> {
    URLSession.shared.dataTaskPublisher(for: mainUrl)
        .map(\.data)
        .decode(type: [Int].self, decoder: decoder)
        .flatMap { self.hackerNewsIdsPublisher(for: $0) }
        .receive(on: RunLoop.main)
        .eraseToAnyPublisher()
}

(Now, you could probably cram all of the above into a single publisher, but I like to keep them small, so each individual publisher is very easy to reason about.)

So, pulling that all together, you have a view model like so:

import Combine

struct HNStory: Codable, Identifiable {
    var id: UInt
    var title: String
    var score: UInt
}

class ViewModel: ObservableObject {
    @Published var stories: [HNStory] = []
    private let networkManager = NetworkManager()
    private var request: AnyCancellable?

    func fetch() {
        request = networkManager.hackerNewsPublisher().sink { completion in
            if case .failure(let error) = completion {
                print(error)
            }
        } receiveValue: {
            self.stories.append($0)
        }
    }
}

class NetworkManager {
    private let decoder = JSONDecoder()

    let mainUrl = URL(string: "https://hacker-news.firebaseio.com/v0/topstories.json")!

    func storyUrl(for id: Int) -> URL {
        URL(string: "https://hacker-news.firebaseio.com/v0/item/\(id).json")!
    }

    func hackerNewsPublisher() -> AnyPublisher<HNStory, Error> {
        URLSession.shared.dataTaskPublisher(for: mainUrl)
            .map(\.data)
            .decode(type: [Int].self, decoder: decoder)
            .flatMap { self.hackerNewsIdsPublisher(for: $0) }
            .receive(on: RunLoop.main)
            .eraseToAnyPublisher()
    }

    // publisher for array of news stories, processing max of 4 at a time

    func hackerNewsIdsPublisher(for ids: [Int]) -> AnyPublisher<HNStory, Error> {
        Publishers.Sequence(sequence: ids.map { hackerNewsStoryPublisher(for: $0) })
            .flatMap(maxPublishers: .max(4)) { $0 }
            .eraseToAnyPublisher()
    }

    // publisher for single news story

    func hackerNewsStoryPublisher(for identifier: Int) -> AnyPublisher<HNStory, Error> {
        URLSession.shared.dataTaskPublisher(for: storyUrl(for: identifier))
            .map(\.data)
            .decode(type: HNStory.self, decoder: decoder)
            .eraseToAnyPublisher()
    }
}

And your main ContentView is:

struct ContentView: View {
    @ObservedObject var viewModel = ViewModel()

    var body: some View {
        Text("Hacker News").font(.headline)
        List(viewModel.stories) { story in
            VStack {
                Text(story.title)
                Text(story.score.description)
            }
        }.onAppear {
            viewModel.fetch()
        }
    }
}
Rob
  • 415,655
  • 72
  • 787
  • 1,044
0

By calling Fetch().getStories, the Fetch class goes out of scope immediately and isn't retained.

I'd recommend making Fetch an ObservableObject and setting it as a property on your view:

@StateObject var fetcher = Fetch()

Then, call:

fetcher.getStories {
  self.stories = stories
}

If you wanted to get even more SwiftUI-ish with it, you may want to look into @Published properties on ObservableObjects and how you can make your view respond to them automatically. By doing this, you could avoid having a @State variable on your view at all, not have to have a callback function, and instead just load the stories into a @Published property on the ObservableObject. Your view will be re-rendered when the @Published property changes. More reading: https://www.hackingwithswift.com/quick-start/swiftui/observable-objects-environment-objects-and-published

jnpdx
  • 45,847
  • 6
  • 64
  • 94