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

Adding new devices #23

Open
andrealufino opened this issue Sep 10, 2024 · 8 comments
Open

Adding new devices #23

andrealufino opened this issue Sep 10, 2024 · 8 comments

Comments

@andrealufino
Copy link
Owner

The idea is to get rid of the old Deviice object and to simplify everything by adding a JSON file with a fixed structure to add new records, as @GregOriol suggested a year ago.
@GregOriol what do you think? I can put this in my pipeline to get this done asap.

@GregOriol
Copy link
Contributor

@andrealufino JSON data could be great, it could be used by other projects/technologies too. Just have to find a way to spend less editing when new devices or formats/types appear (eventhough it's only once or twice a year).

@andrealufino
Copy link
Owner Author

Yes, so the idea is this:

  1. Remove deprecated Deviice class
  2. Introduce a JSON file with a standard format
  3. Introduce a formatter for the JSON
  4. Have a single Device class with all the info automatically taken from the json

Question: what about the enums for the screen size, connectivity...? Do we want to keep them or not? I'm not sure they're really useful, but with no doubt they helps in case someone needs to filter values or stuff like that. Not something that is often used I think.

@andrealufino
Copy link
Owner Author

@GregOriol I'd also get rid of the Cocoapods honestly, I think SPM is the standard and should be always used instead of pods.

@GregOriol
Copy link
Contributor

@andrealufino I personally don't really use the enums, just actualModel.marketingName and screenSize.rawValue, and it's for some debugging/support help, not meaningful code

However I'd keep Cocoapods: still using it on many projects and no plans to upgrade old projects

@andrealufino
Copy link
Owner Author

Ok, let's keep Cocoapods so. I will try to code something on a separate branch and let's see :)

@andrealufino
Copy link
Owner Author

@GregOriol Just wanting to share two different format for the json:

First one:

[
    {
        "identifier": "iPhone17,3",
        "screenSize": 6.3,
        "year": 2024,
        "marketingName": "iPhone 16 Pro",
        "debugName": "iphone16pro"
    },
    {
        "identifier": "iPhone17,4",
        "screenSize": 6.9,
        "year": 2024,
        "marketingName": "iPhone 16 Pro Max",
        "debugName": "iphone16promax"
    },
...

Second one:

{
    "iPhone17,4": {
        "year": 2024
    },
    "iPhone17,3": {
        "year": 2024
    }
}

Please, ignore the attributes for now, this is just a test. I don't know the difference in terms of performance, if there's any. Do you have an idea about this?

@GregOriol
Copy link
Contributor

GregOriol commented Sep 11, 2024

@andrealufino The first one might be more generic, something that would cover more usages like someone willing to find all devices with a given screen size, while the second will be faster to use for the most common use case of accessing device information based on the identifier.

Both are easy to process in the most common languages Swift, javascript, php, ...

Personally my use case is the second one, and the first structure would require to loop over the list so quite longer processing times... though it's not really an issue as this code is only executed rarely in my projects.

So no clear winner here :-) but I'd go for the second one as it's what the library is about!

@andrealufino
Copy link
Owner Author

@GregOriol I had some time to sum everything up and I developed what I think the final library should be. It's on the branch feature/json (that is linked to this issue). This is what the new library will have:

  • Model that refers to the device model, I think this is useful to have
  • Device that represents the entire object

I would keep the Family, but I'd get rid of ScreenSize, Groups, Deviice, Mapper and Identifier. This will make the package a lot lighter and to add a new device it will require just two steps:

  1. Add it to the json
  2. Add the model in the Model enum

Please note that, for now, the Model and Device classes I'm talking about are called NewModel and NewDevice.

Waiting for your feedback 🙂

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

When branches are created from issues, their pull requests are automatically linked.

2 participants