Welcome, guest! Please login or register.

    * Shoutbox

    RefreshHistory
    • B50: Sever error what happen
      May 22, 2018, 08:23:40 PM
    • The Soul: 10 years later
      May 21, 2018, 05:23:10 PM
    • The Soul: wow people are still here?
      May 21, 2018, 05:19:27 PM
    • Wilkooo: and apologies to the OG forum guys on behalf of that fudgewit dr house that pretty much killed this entire domain
      May 21, 2018, 04:14:28 PM
    • Wilkooo: shoutout to all those that played back in the beginning, all new people that hang around now seem to be pretty braindead
      May 21, 2018, 04:13:54 PM
    • Wilkooo: born sep 2016 - died whenever pink eggs resigned
      May 21, 2018, 04:11:29 PM
    • Wilkooo: Rip moparscape rsps
      May 21, 2018, 04:11:10 PM
    • puta loca: or what section can i ask this
      May 21, 2018, 05:45:08 AM
    • puta loca: does someoen has platinum ps v2 files?
      May 21, 2018, 05:44:59 AM
    • w azza 3: server down??
      May 21, 2018, 05:07:47 AM
    • charmie: rippppppppppppppppppppppppppppppppppppppppppppppp
      May 20, 2018, 09:03:41 PM
    • Tesco Value: eco reset? :o
      May 20, 2018, 08:54:27 PM
    • Tesco Value: aw is server down? :P
      May 20, 2018, 08:54:03 PM
    • mandmgalaxy: is the game down?
      May 20, 2018, 08:05:07 PM
    • bliss death: i believe 95% of the community disliked this change heavily as it came out of nowhere. and the fact you clear ironmen banks as well. terrible change. disappointed.
      May 20, 2018, 06:08:36 PM
    • bliss death: just wondering when the server is gonna be fixed and reverted
      May 20, 2018, 06:07:36 PM
    • Saltyspade10: I'll be back ;)
      May 15, 2018, 04:43:53 PM
    • Nunubuffs:[link]
      May 15, 2018, 12:06:25 PM
    • Nunubuffs: .info god
      May 15, 2018, 12:05:40 PM
    • Nunubuffs: Www.scaperune
      May 15, 2018, 12:05:25 PM

    Author Topic: soldout  (Read 540 times)

    0 Members and 1 Guest are viewing this topic.

    OfflineRandQm

    • Member
    • ****
    • Posts: 4,220
    • Thanks: +0/-0
      • View Profile
    soldout
    « on: September 15, 2014, 01:37:14 PM »
    soldout
    « Last Edit: September 15, 2016, 12:38:11 PM by RandQm »
    soldout

    contact me on [email protected] if you're looking for me
    RS2Ad banner

    Offlinetyb97

    • Member
    • ****
    • Posts: 6,195
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #1 on: September 15, 2014, 01:42:01 PM »
    I'm not sure how efficient or safe creating a new thread for each event is. However, nice use of parallelism.
    what if i want to stick it in? you got a problem with that?
    RS2Ad banner

    OfflineRandQm

    • Member
    • ****
    • Posts: 4,220
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #2 on: September 15, 2014, 01:49:41 PM »
    I'm not sure how efficient or safe creating a new thread for each event is. However, nice use of parallelism.
    I assume you mean creating a new thread for every parallel event as the normal ones run on the same thread?
    As you can see I'm catching the exception if there would be to many threads, but according to my research a simple computer can run
    over a K threads so I don't think that would be a problem very quick.
    soldout

    contact me on dai[email protected] if you're looking for me
    RS2Ad banner

    Offlinetyb97

    • Member
    • ****
    • Posts: 6,195
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #3 on: September 15, 2014, 02:16:47 PM »
    I'm not sure how efficient or safe creating a new thread for each event is. However, nice use of parallelism.
    I assume you mean creating a new thread for every parallel event as the normal ones run on the same thread?
    As you can see I'm catching the exception if there would be to many threads, but according to my research a simple computer can run
    over a K threads so I don't think that would be a problem very quick.
    I didn't know those numbers, but looks good.

    The only other thing I see is it just seems a bit verbose to me. Couldn't you simplify things like this
    Code: Java
    1.         /**
    2.          * Disposes the event manager.
    3.          */
    4.         publicvoid dispose(){
    5.                 synchronized(parallelThreads){
    6.                         for(Iterator<EventThread> iterator = parallelThreads.iterator(); iterator.hasNext();){
    7.                                 ((EventThread)iterator.next()).dispose();
    8.                                 iterator.remove();
    9.                         }
    10.                 }
    11.                 parallelThreads.clear();
    12.                 logger.info("Event manager has been disposed.");
    13.         }
    14.  

    with using lambdas, parallelStream(), forEach(), etc?

    https://stackoverflow.com/questions/16635398/java-8-iterable-foreach-vs-foreach-loop

    Did some extra research since I haven't personally used them in this way yet, only seen it.
    what if i want to stick it in? you got a problem with that?

    OfflineDavidi2

    • Member
    • ****
    • *
    • Posts: 23,272
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #4 on: September 15, 2014, 02:53:24 PM »
    Code: Java
    1.                 if(event.getDelay()<=0){
    2.                         logger.warning("Delay must be positive.");
    3.                         return;
    4.                 }
    You should instead check this value on the Event initialization in the constructor and throw a potential IllegalArgumentException

    Also I don't really like the design of using a list to wrap the event properties. Right now you are using an enum list to basically wrap 3 booleans because you would rather have a List than those booleans themselves. While it make look more elegant/cool isn't really a good reason to do it. If you realllly don't want to just add those booleans then at least use java.util.Properties

    I don't know a lot about the specifics with threads and such, but at first glance it also looks like this would be a crazy cpu hog, because it never sleeps. You should calculate how long each thread can sleep during the tick

    Lastly, is there any reason you have 'current' as a persistent variable outside of the tick method for events? It just seems like it's unneeded.



    OfflineRandQm

    • Member
    • ****
    • Posts: 4,220
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #5 on: September 15, 2014, 03:38:25 PM »
    Code: Java
    1.                 if(event.getDelay()<=0){
    2.                         logger.warning("Delay must be positive.");
    3.                         return;
    4.                 }
    You should instead check this value on the Event initialization in the constructor and throw a potential IllegalArgumentException

    Also I don't really like the design of using a list to wrap the event properties. Right now you are using an enum list to basically wrap 3 booleans because you would rather have a List than those booleans themselves. While it make look more elegant/cool isn't really a good reason to do it. If you realllly don't want to just add those booleans then at least use java.util.Properties

    I don't know a lot about the specifics with threads and such, but at first glance it also looks like this would be a crazy cpu hog, because it never sleeps. You should calculate how long each thread can sleep during the tick

    Lastly, is there any reason you have 'current' as a persistent variable outside of the tick method for events? It just seems like it's unneeded.
    Thanks for the bunch of feedback, once I made changes I will update the topic.
    There's no reason why the current variable is outside the tick other than an oversight.
    About the properties, it's not because it's more fancy but because it is easier to read/use for a layman imo.



    I'm not sure how efficient or safe creating a new thread for each event is. However, nice use of parallelism.
    I assume you mean creating a new thread for every parallel event as the normal ones run on the same thread?
    As you can see I'm catching the exception if there would be to many threads, but according to my research a simple computer can run
    over a K threads so I don't think that would be a problem very quick.
    I didn't know those numbers, but looks good.

    The only other thing I see is it just seems a bit verbose to me. Couldn't you simplify things like this
    Code: Java
    1.         /**
    2.          * Disposes the event manager.
    3.          */
    4.         publicvoid dispose(){
    5.                 synchronized(parallelThreads){
    6.                         for(Iterator<EventThread> iterator = parallelThreads.iterator(); iterator.hasNext();){
    7.                                 ((EventThread)iterator.next()).dispose();
    8.                                 iterator.remove();
    9.                         }
    10.                 }
    11.                 parallelThreads.clear();
    12.                 logger.info("Event manager has been disposed.");
    13.         }
    14.  

    with using lambdas, parallelStream(), forEach(), etc?

    https://stackoverflow.com/questions/16635398/java-8-iterable-foreach-vs-foreach-loop

    Did some extra research since I haven't personally used them in this way yet, only seen it.
    I have used lambdas when programming with Linq, though it can look clean I find the readability not that good, especially for people who don't know lambda's so I prefer not to use them.
    soldout

    contact me on [email protected] if you're looking for me

    OfflineDavidi2

    • Member
    • ****
    • *
    • Posts: 23,272
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #6 on: September 15, 2014, 03:51:47 PM »
    About the properties, it's not because it's more fancy but because it is easier to read/use for a layman imo.
    What do you mean? If you mean getProperties().contains(IMMEDIATE) is easier to read/use than isImmediate() then I disagree :P

    OfflineRandQm

    • Member
    • ****
    • Posts: 4,220
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #7 on: September 15, 2014, 04:07:09 PM »
    About the properties, it's not because it's more fancy but because it is easier to read/use for a layman imo.
    What do you mean? If you mean getProperties().contains(IMMEDIATE) is easier to read/use than isImmediate() then I disagree :P
    Users would only need to put properties in as parameter when creating a new event, EventProperty.SOMETHING is easier than getting the correct string for the util.properties
    soldout

    contact me on [email protected] if you're looking for me

    OfflineDavidi2

    • Member
    • ****
    • *
    • Posts: 23,272
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #8 on: September 15, 2014, 04:08:30 PM »
    Ok, but what about the other part of what I said, as in not actually using a list/map at all, and just using separate booleans?

    OfflineRandQm

    • Member
    • ****
    • Posts: 4,220
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #9 on: September 15, 2014, 04:14:33 PM »
    Ok, but what about the other part of what I said, as in not actually using a list/map at all, and just using separate booleans?
    Yeah the list is maybe not necessary


    edit: Updated the code on the topic
    « Last Edit: September 15, 2014, 04:27:47 PM by RandQm »
    soldout

    contact me on [email protected] if you're looking for me

    OfflineLothy

    • Member
    • ****
    • *
    • *
    • Posts: 7,006
    • Thanks: +0/-0
      • View Profile
    <&Speljohan_> i wouldnt want to live in a society where Mopman isnt monitored 24/7

    OfflineAshi

    • Member
    • ****
    • Posts: 3,584
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #11 on: September 17, 2014, 07:49:50 AM »
    http://docs.oracle.com/javase/tutorial/essential/concurrency/executors.html
    Would that be appropriate for events though. I though executors are best for task systems because you're not creating a new thread for each task.

    OT: It's good practice to use braces when you can.


    Edit: Just realised that this isn't an event system. It's a task system.

    OfflineRandQm

    • Member
    • ****
    • Posts: 4,220
    • Thanks: +0/-0
      • View Profile
    Re: Timed Event System - Requesting feedback
    « Reply #12 on: September 17, 2014, 11:23:38 AM »
    http://docs.oracle.com/javase/tutorial/essential/concurrency/executors.html
    Would that be appropriate for events though. I though executors are best for task systems because you're not creating a new thread for each task.

    OT: It's good practice to use braces when you can.


    Edit: Just realised that this isn't an event system. It's a task system.
    Yeah I realized it as well while reading your first line lol
    soldout

    contact me on [email protected] if you're looking for me

     

    Copyright © 2017 MoparScape. All rights reserved.
    Powered by SMFPacks SEO Pro Mod |
    SimplePortal 2.3.5 © 2008-2012, SimplePortal