Request for structure review

[josefnpat]

So, for Vapor1.x, I'm looking for suggestions for the architecture of the library.

Check out this document which describes how the rest of the framework, along with a pseudocode example.

The is entirely OOP, but without a dependent library.

Currently I have the main frame out for everyone to see, and I am open to ideas & suggestions before I start filling out the functions and testing everything. (help would obv. be appreciated when that time comes.)

[pablomayobre]

I like the overall look!

The pseudocode uses arg[1] I'm guessing that arg[1] is passed through the console right?

vapor arg[1]

I would like to see the relationship between the different types of classes, let's say game:getReleases returns a table (or collection) with release objects, just an example.

But for that I guess you should expose more methods of each class right? haha sorry if I'm getting ahead.

Also about download *state I think it should have a state value as you wrote in there, but also a percentage value (from 0 to 1?) In order to show progress vars. That would be seriously nice

The rest is all fine with me :smile:

[josefnpat]

Thanks for looking over this!

the arg[1] is indeed pseudocode, and is just a cli example of how a "download and play" vapor script could be written.

What kind of relationship with the collection and release objects are you talking about? When you get a release object, you will be able to use all of the functions it provides (and that download provides since it polymorphically extends it)

e.g. if you wanted to download the stable release of every game you would;

v = vapor.new() -- create the vapor object
while not done do
  v:update() -- let vapor manage it's threads, downloads, etc
  local done = true
  for _,game in pairs(v:getGameCollection():all()) do --- for each game
    for _,release in pairs(game:getReleases():all()) do -- for each release
       -- if the stable release is not downloaded
       if release:getStable():getStatus() == vapor.status.uninitialized then
         -- tell vapor you want this data (e.g. start the download)
         release:getStable():updateData()
      end
      -- if the stable release is not ready
      if release:getStable():getStatus() ~= vapor.status.ready then
        done = false
      end
    end
  end
end

Adding download percentage would be really nice, and with the current structure (assuming we can figure out how to get that number via the vapor.util.async function I plan on porting from Vapor0.x) it would be rather easy to extend the download object to have a :getDownloadProgress function.

[Bobbyjoness]

Can I be picky and say you should use a folder for all the classes?

[josefnpat]

@Bobbyjoness I think that's a good idea.

[josefnpat]

@Positive07 added a uninit state: https://github.com/josefnpat/vapor/commit/37fa77016e467ab07fbab46890a5695f28a54e49

[Bobbyjoness]

Um about the download percentage. We can have a thread compare the current file size to the expected end size and make that a percentage. We can have a dedicated download percentage thread for that. I hope this makes sense.

[pablomayobre]

@josefnpat When I was talking about releases I was saying just that hahaha, Just got confused with the doc a little, now it's all clear.

@Bobbyjoness I dont think that is a good idea, When I worked on something similar I made little request asking for chunks of the code, that way I could monitor how many chunks I had downloaded and how many chunks were missing (you can get the size of a file with a HEAD request)

I dont know if this is nice enough, since it requires multiple requests but it worked

[Bobbyjoness]

Would that require breaking it up into chunks on the server side or what? I can see that being complicated and taking longer to download. But idk how it would work so idk if it is or isn't complicated but sure seems like it.

[pablomayobre]

Nope, there is a flag (or header, not sure the name) that allows you to specify the start byte and how many bytes you wanna download... Gonna look it up. Plus it is mainstream HTTP and so it is already supported by LuaSocket. I dont think it would be slower (but it may be, just a little I guess)

[josefnpat]

As @Positive07 mentioned on IRC, I will consider adding state check's via function calls on anything that extends from download, e.g.:

download:isUninitialized
download:isReady
download:isDownloading
download:isHashing
download:isProcessing
download:isFail()

The only issue I have with that at this moment is that I don't really know all the states that should be available at this moment.

[pablomayobre]

Other option, having a table called vapor.status that holds all the status, then a method called is() for the download class that expects one argument, a vapor.status entry, and returns true if in the specified status... May be horrible and the same as the:

if release:getStable():getStatus() == vapor.status.uninitialized then

Just as an idea

[josefnpat]

@Positive07

So something like this?

if release:getStable():getStatus("ready") then

Or worse:

if release:getStable():getStatus(vapor.status.ready) then

[Bobbyjoness]

Why would that last one be an option? Lol

[pablomayobre]

First one hahaha though you could get the status from the vapor.status table

vapor.status = {
     "unititialized";
     "downloaded";
     ...
}

[josefnpat]

I'm being a little overzealous with performance here, but I was hoping to avoid strings, since comparing numbers is faster.

If you think that using strings to improve API clarity would much outweigh the benefit of indexing them as I am currently doing, tell me. I'd be down to that change.

[pablomayobre]

Whatever is better for you... It's the same, plus I would use those strings to pass them to the function, say:

if dowload.getStatus(vapor.status[1]) then
    ...

[pablomayobre]

Both the isSomething methods and the getStatus methods are both clear, and clearer than the getStatus() == stuff

[josefnpat]

@Positive07 How would you feel if we did "both" so to speak and just overload getStatus to check against integers and strings? That way the user can do either.

I do want to keep getStatus(nil) so that users can easily translate statuses into different languages.

e.g.

if download:getStatus(vapor.status.ready) then
  print( vapor.util.lang.translate("en",download:getStatus())
end

and

if download:getStatus("ready") then
  print( vapor.util.lang.translate("gr",download:getStatus())
end

[pablomayobre]

Sounds nice, was thinking about the same thing. so

download.getStatus(0) == download.getStatus("uninitialized")
download.getStatus() == "uninitialized" or vapor.status.unititialized