Tweakers maakt gebruik van cookies, onder andere om de website te analyseren, het gebruiksgemak te vergroten en advertenties te tonen. Door gebruik te maken van deze website, of door op 'Ga verder' te klikken, geef je toestemming voor het gebruik van cookies. Je kunt ook een cookievrije versie van de website bezoeken met minder functionaliteit. Wil je meer informatie over cookies en hoe ze worden gebruikt, bekijk dan ons cookiebeleid.

Meer informatie

Door , , reacties: 91, views: 13.439 •

De eerste iteratie van 2013 zit erop en hoewel alle devvers lekker vakantie hebben opgenomen en het erg rustig was op kantoor, zijn er toch nog 83 tickets weggewerkt.

Cookiemuur

Ook wij ontkomen niet aan de cookiewetgeving. De balk die wij eerst toonden, was helaas niet voldoende om aan alle eisen van de wet te voldoen en dus moest deze worden aangepast. Het resultaat hebben jullie allemaal al gezien ;)

Vooral kleine dingetjes

Naast de cookies zijn er vooral kleine dingen opgelost en zijn we begonnen met een aantal zaken die nog steeds in ontwikkeling zijn.

Op het forum is de tijdsnotatie van oude topics aangepast. Voorheen hadden oude topics een tijdsnotatie als '10-'05'; dit wordt nu weergegeven als: '18-10-2005'. Hierdoor wordt niet alleen de dag zichtbaar, maar is ook de overzichtelijkheid van de listings verbeterd.

In de karmaberekening hebben we een aanpassing gedaan om de schaalverandering van de reviews te verwerken. Sinds Tweakers 7 worden reviews niet meer met 1 tot 5 gewaardeerd, maar met een score van 0 tot 3. Nu krijg je weer het juiste karma voor een review die als +3 is beoordeeld.

Verder zijn er nog wat bugjes in V&A opgelost. Zo kon je soms een doorlooptijd opgeven, terwijl de advertentie geen veiling was en werd het startbod in sommige gevallen op 0 gezet als je een veiling bewerkte.

Codekwaliteit

Om de codekwaliteit in de gaten te houden en te verbeteren hebben we de code-quality-tool Sonar geïnstalleerd.

Sonar

We hebben nu een beter overzicht van de kwaliteit van de code en worden gewezen op plekken waar we misschien naar moeten kijken. Zo laat Sonar plekken zien met een hoge cyclomatische complexiteit of waar 'bad practices' gebruikt worden. De tool moet nog wel wat gefinetuned worden. Zo is er een aantal zaken die Sonar nu nog als major beschouwt, maar die bij ons wel zijn toegestaan. Een if statement zonder braces wordt door Sonar altijd als major issue aangeduid, terwijl wij dat in sommige gevallen wel gebruiken.

Reacties (91)

Reactiefilter:-191091+166+210+30
Mooi dat tweakers steeds verbeteringen aan blijft brengen. Alleen jammer dat die Cookie waarschuwing verplicht is. Ik word nogal moe van al die waarschuwingen op internet.
Maar zodra je eenmaal de consent bij ons gegeven hebt, zul je die niet meer zien. :) Mocht je uiteraard vanaf een ander device komen waar je niet bent ingelogd, dan natuurlijk wel :+
Mag je die consent koppelen aan een user-account en dan over verschillende apparaten toepassen, ja?
moet je nog wel even inloggen...op de apparaten..en als je al ingelogt bent dan accepteer je al cookies...
Tenzij je dus al op die apparaten was ingelogd. Maar die groep wordt natuurlijk nu steeds kleiner.
Een if statement zonder braces wordt door Sonar altijd als major issue aangeduid, terwijl wij dat in sommige gevallen wel gebruiken.
Waarom?
Omdat het in sommige situaties leesbaarder uitpakt.
Bij ons is het geen enkele geval dat een if statement op 1 regel, zonder braces iets leesbaarder maakt...

Je wordt er dan ook zwaar op afgerekend door je team hier }>

[Reactie gewijzigd door Phoenix_the_II op 8 januari 2013 14:10]

Als dat de dingen zijn die bij jullie zwaar worden aangerekend dan zit er bij je team een steekje los.
Nee, we willen juist door 1 coding stijl aan te houden, de gehele code basis leesbaar te houden voor iedereen. Coding standards zijn regels, eentje waar veel waarde gehecht aan wordt.
Het maakt in principe niet uit wat je kiest, als je hele team maar hetzelfde doet. Dat kan een tool als Sonar of Codesniffer afdwingen.
Inderdaad, consistentie is normaal gezien het belangrijkste argument om een bepaalde stijlregel in te voeren.

Waar ik werk maken we ook volop gebruik van Sonar. Om een bepaalde codestijl af te dwingen maken we ook in Eclipse gebruik van de Checkstyle plugin die in Sonar draait. We hebben 1 à 2 kleine wijzigingen gedaan aan de regels, maar over het algemeen helpt dit enorm.

Over het laatst hadden we een oud project dat nog van voor de introductie van Checkstyle gemaakt was en die code was zo inconsistent als het maar kon omdat iedereen er een eigen stijl op gebruikte. Dat zullen we gelukkig niet voorhebben met een van onze nieuwe projecten waarbij de codestijl vast staat (en gevalideerd wordt vooraleer er gebuild wordt).
Nee, we willen juist door 1 coding stijl aan te houden
Dat is prima (en terecht imho), maar ik reageer op de door jouw gekozen nuancering. "Zwaar afrekenen" is niet iets dat je toepast bij een redelijk kleine regel als verplicht braces bij een if. Redenen om zwaar af te rekenen zijn mijns inziens zaken als het veelvuldig gebruik van goto. Als je bij de missende braces al collega's zwaar aan het afrekenen bent dan klopt er iets niet.

[Reactie gewijzigd door .oisyn op 8 januari 2013 18:16]

Als dat de dingen zijn die bij jullie zwaar worden aangerekend dan zit er bij je team een steekje los.
Nou nou, wat een overdreven reactie is dat weer zeg! Ik vind het persoonlijk ook niet erg netjes om if statements zonder braces te gebruiken. En het zal me niks verbazen als bij ons de checkstyle plugin het build-proces ook stopt in zulke gevallen. Zit er dan bij ons ook een steekje los? Heeft meneer .oisyn en/of T.net de waarheid in pacht?
[...]

Nou nou, wat een overdreven reactie is dat weer zeg! Ik vind het persoonlijk ook niet erg netjes om if statements zonder braces te gebruiken. En het zal me niks verbazen als bij ons de checkstyle plugin het build-proces ook stopt in zulke gevallen. Zit er dan bij ons ook een steekje los? Heeft meneer .oisyn en/of T.net de waarheid in pacht?
De nadruk ligt natuurlijk op het "zwaar afrekenen".
Nou nou, wat een overdreven reactie is dat weer zeg!
.oisyn in 'plan: Development-round-up - Iteratie #28' (staat nota bene direct boven jouw post)
Het "zwaar afrekenen" is wat hier overdreven is, en waar ik op reageer. En natuurlijk chargeer ik, net zoals de persoon waar ik op reageer.

[Reactie gewijzigd door .oisyn op 9 januari 2013 22:57]

IMHO is dat bad practice.
Heb te vaak meegemaakt dat 'iets' niet werkte omdat men vergeten was om de braces toe te voegen op het moment dat men de if statement aan het uitbreiden was.

Daarom zeg ik altijd braces gebruiken bij een if statement
als er na de IF maar 1 regel code komt en geen ELSE, gebruik ik geen braces. Volgt er een ELSE, gebruik ik het wel. Dit is voor mij het meest leesbare. Maar om het nou als Major aan te duiden. Dat is wat overbodig lijkt mij.
Dan is dat een fail van degene die het if statement uitgebreid heeft, niet van degene die het zonder braces geschreven heeft.

Ik laat de braces altijd weg als er maar 1 statement volgt (en inderdaad, geen else). Al spring ik wel altijd 1 niveau in tenzij het allemaal makkelijk op 1 regel past.
Helemaal gelijk.

Ik zet mijn korte ifs, zonder else, die op 1 regel gaan ook niet met haakjes.
Veel duidelijker IMO.

Maar smaken verschillen. Gelukkig zijn er zat formatterings tooltjes en is dit eigenlijk een non-discussie.
Ik zet ze wel, op het moment van schrijven een zeer kleine moeite en het toont altijd netjes aan hoe groot het statement is, zelfs al is het maar 1 regel.
Dat hangt helemaal van de situatie af eigenlijk. Zelf vind ik het juist prettig dat je met eenvoudige ifs geen haakjes gebruikt zodat je meteen weet dat het daar al ophoudt. Diep geneste ifs heb ik dan vaak wel weer met haakjes anders wordt het wel erg onleesbaar.
diepgeneste ifs? wat is bij jou diep? anders zou ik toch eens kijken naar de ifstatements of je ze niet kunt samenvoegen...
Ik weet niet waarom

if (val)
return val;

minder goed leesbaar is dan

if(val) {
return val;
}

Het eerst is JUIST mooier en overzichtelijker
Tja, je kan van het feit dat er meerdere returns in een methode staan ook weer een issue maken.

Het gaat er gewoon om dat de code consistent is. Welke vorm je dan kiest is niet eens zo belangrijk. Het scheelt een hoop "inleestijd" als je de code van een collega inkijkt en die is hetzelfde geformatteerd als die van jezelf.
Ik maak bijvoorbeeld altijd een punt van meerdere return statements (zeikerds heb je in elk bedrijf :+ )

Ik maak wel als uitzondering recursieve functies waarbij ik onderscheid maak in de return voor de base case en de recursieve stap.

Dus ook voor mij blijft dit discutabel en is het raar dat Sonar dit als 'major' aanvinkt.

Enfin, kende sonar niet, zal eens poken om het hier te installeren :Y)
Vind ik ook (mits je wel inspringt, anders vind ik het wel onduidelijk). Maar in frontpage comments kan je niet inspringen dus ik denk dat dat weggevallen is :)
hier is het inderdaad niet zo'n issue, maar er kunnen meerdere returns worden toegevoegd, en als je een flinke rits code hebt en je hebt geen braces, moet je zoeken waar de statement ophoud, dat kan een tijdrovende klus zijn. Met braces kun je in één oogopslag zien waar iets begint, en waar het eindigt. Kleine moeite om het dan ook bij one- en twoliners als jouw voorbeeld toe te passen.

Maar het blijft geneuzel, het is een manier om een lap code leesbaarder te maken. Persoonlijk prefereer ik:

if(val) {
return(val)
return(val)
return(val)
}

Maar het blijft voorkeur. Voor mij blijft het overzichtelijk, en tussen statements altijd een witregeltje. :)

[Reactie gewijzigd door naarden 4ever op 8 januari 2013 23:51]

Mensen hebben altijd een mening waarom iets duidelijk of is overzichtelijker.
{ op de zelfde nieuwe regel is een kwestie van smaak, maar als je verkiest om alles op één regel te schrijven vraag je gewoon om problemen.

[code]
if (conditie) return foo;
[/code]

Lijkt misschien duidelijk maar dit is dit niet. Zelf heb ik dit ook vele jaren gedaan, maar ben hier toch vanaf gestapt.

Bekijk dit uit het oogpunt van executie.
Je ziet een if statement, een conditie, als die conditie voldoet voer je iets uit.
Je gaat dus een niveau diepen, daarom springen we code in omdat dit namelijk buiten 'de main' flow valt. Als je dit op één regel zet breek je deze denk conventie.

Daarnaast is de meeste code langer dan een return foo;, en word je code veel langer en complexer om te begrijpen en debuggen. Leesbaarheid en conventies helpen om de code sneller en makkelijker te begrijpen.

Het toppund wat ik een keer heb gezien was iemand die een functie declaratie van in cq 40 regels op één regel had neer 'gedonderd'. HOE IN GODSNAAM KAN DIT NU LEESBAAR ZIJN!

Zie ook https://www.youtube.com/watch?v=_EANG8ZZbRs waarin hij uitlecht waarom we programmeren op een manier die duidelijk en prettig is.
Wij zetten de braces op de volgende regel en in sommige gevallen, waar de complexiteit laag is, wordt het nog wel eens gebruikt om 4 regels te 'besparen' wat de leesbaarheid kan vergroten:

Zie dit voorbeeld: http://pastebin.com/qfkvGZbe

Met geneste if's e.d. worden braces wel altijd gebruikt.

[Reactie gewijzigd door JoostBaksteen op 8 januari 2013 14:09]

In dit voorbeeld kan je ook gewoon "return bar" doen.
Of "return bar ? true : false;" :)
Grapje neem ik aan? :X
Hint: bar is al true, dus waarom true returnen als bar al de waarde is die je wilt returnen :)
Nergens staat dat de waarde van bar true is. Het zou zo maar de evaluatie van een operator overload op een object kunnen zijn die true terug geeft ; het object terug geven is dan niet gelijk aan true terug geven ;)

[Reactie gewijzigd door Fealine op 8 januari 2013 17:37]

Uhm, is dat niet: return (bar) ? Of ben ik nu een voorbeeld uit het verband aan het trekken?
Je kan inderdaad ook gewoon "return bar;" doen. Maar het voorbeeld gaat erom dat de eerste leesbaarder is als de tweede of de derde volgens sommige. ;) Iets waar ik het wel mee eens ben eigenlijk.
Of gewoon

if (foo) { return true; }
return false;
Sonar en Checkstyle/PMD/FindBugs heeft soms behoorlijk rare dingen die major zijn inderdaad.
Zo mag een methode niet langer dan 20 regels zijn standaard en maar max 5 argumenten hebben. Tja, er zijn gevallen waar het gewoon niet praktisch is om dat niet te doen.
Nog zo'n voorbeeld: System.out.println() mag niet (gebruik logging is de kreet) - maar als je een commandline utility hebt is dat wel handig om de usage te printen.
Ook wij ontkomen niet aan de cookiewetgeving. De balk die wij eerst toonden, was helaas niet voldoende om aan alle eisen van de wet te voldoen en dus moest deze worden aangepast. Het resultaat hebben jullie allemaal al gezien ;)
Het is wel ontzettend jammer dat die cookiepopup de html achter de pagina niet alvast inlaad.. Ik was bezig met een Tweakers windows 8 applicatie voor de AppRace die ik uiteindelijk niet gereleased heb door deze popup. (Ik had reacties in mijn applicatie weergegeven, maar dit werkt niet meer door de popup)
Een if statement zonder braces wordt door Sonar altijd als major issue aangeduid, terwijl wij dat in sommige gevallen wel gebruiken.
Haha, deze discussie heb ik toevallig gister nog met iemand gevoerd. Het is gewoon een style van programmeren, maar braces gebruiken is altijd aan te raden (Vind ik persoonlijk overzichtelijker).
Helemaal mee eens, was ook mijn feedback toen de .plan over de nieuwe cookiepopup werd gepubliceerd. Ook moet er eens goed worden gekeken naar deze popup om mobiele devices want daar is dit soms echt dramatisch.
Het is wel ontzettend jammer dat die cookiepopup de html achter de pagina niet alvast inlaad.. Ik was bezig met een Tweakers windows 8 applicatie voor de AppRace die ik uiteindelijk niet gereleased heb door deze popup. (Ik had reacties in mijn applicatie weergegeven, maar dit werkt niet meer door de popup)
We kunnen nu eenmaal niet eenvoudig een site zonder (third-party) cookies serveren. Als tijdens het verschijnen van de cookiemuur in de achtergrond de pagina geladen zou worden, zou je al cookies kunnen krijgen voordat je hiervoor toestemming hebt gegeven.
Kunnen jullie uitleggen hoe jullie het doen met de cookie wall mbt de zoekmachines, en andere bots? Wil ook graag een cookiewall implementeren. Nu kan ik google bot etc wel excluden, maar wat als google zelf langskomt met een nieuw ip of andere googlebot wordt hij dus geblokkeerd door een cookiewall.
Totale chaos op T.net HQ op dit moment. Hier had niemand aan gedacht :P

zal wel niet
ACM in 'plan: Tweakers komt met nieuwe implementatie cookienotificatie'
We hebben de melding sowieso alleen voor Nederlandse ip's, dat scheelt al een hoop gedoe. Daarnaast sluiten we ons bekende ipranges van crawlers ook uit van de melding, hoewel er vast nog wel af en toe een robot doorheen valt die 'm wel gaat krijgen...
Interessant, ik woon toch niet in NL, maar heb ook die cookie-melding gekregen (Oostenrijk). Of dat IP-filter dus zo goed werkt....
Jij hebt waarschijnlijk alleen de 'balk' onderin, niet de melding die je zal moeten accepteren voordat je überhaupt op de site kan komen.
In principe is deze cookie implementatie alleen zichtbaar voor Nederlandse IP adressen. Dus buitenlandse IPs krijgen de "oude" cookie balk aan de onderkant van de pagina. Daarnaast hebben we inderdaad nog een lijst aan ip adressen die ge-whitelist staan om te kunnen crawlen.
Hoe zit het eigenlijk met upcoming nieuws? Dit was ook in 1 keer weg, aangezien je ervoor Karma kon inleveren, kan ik me goed voorstellen dat deze vraag voor meerdere mensen geldt. Ik heb er al een tijdje en ticket voor open staan. Ik kan me goed voorstellen dat deze niet op de voorpagina van de to-do list staan, maar een reactie had ik op zich wel verwacht van T.net:
[Feat] Karmastore upcoming nieuws
Upcoming news zal (iig voorlopig) niet terugkomen. Er waren maar een paar gebruikers ervan, wat te weinig was om er veel tijd en moeite in te steken. Ingeleverde karma is gewoon weer teruggegeven. Mbt het ticket: we kunnen helaas niet overal direct op reageren.
Upcoming news zal (iig voorlopig) niet terugkomen. Er waren maar een paar gebruikers ervan, wat te weinig was om er veel tijd en moeite in te steken.
Wel jammer, vond het altijd wel handig (ook om te zien wat er al gesubmit was).
Bedankt voor de update, jammer dat het niet meer door gaat.

Ik veracht inderdaad geen reactie binnen 2 weken, vandaar vreemde reactie:
we kunnen helaas niet overal direct op reageren.
Datum van ticket: Woensdag 24 oktober 2012 16:52
Datum van vandaag: 08 januari 2013 ;)
Hmm, ik hoop dat jullie de unittest metric nog niet aangesloten hebben in Sonar anders maak ik me diep zorgen :P. Tenslotte code zonder testen bestaat niet.

[Reactie gewijzigd door retakenroots op 8 januari 2013 14:03]

Klopt, de daadwerkelijke coverage is nog altijd niet heel bijzonder, maar wel een stuk beter dan op dit plaatje te zien is :)
Het 'geen toegang geven bij niet accepteren van de cookie-ding' is toch geen onderdeel van de wet?
Gedeeltelijk wel. Als jij geen toestemming geeft dan moeten wij een site zonder cookies aanbieden en dat is voor ons technisch vrij ingewikkeld, dus bieden wij geen cookieloze site aan.
Daar had ik eerder al discussie over, technisch an sich nog haalbaar, commercieel gezien totaal niet :P
Jewel, aangezien tweakers niet kan garanderen dat er geen cookies worden geplaatst door derde. Een plaatje op het forum is al genoeg om cookies van een externe imagehost in je browser te krijgen. Zelfs al zou je alle reclames waar iedereen zo over struikelt verbannen.

Dus het is of toestemming vragen, of geen toegang geven omdat je geen garanties af kan geven anders. Daarbij heb je prima zelf de optie om 3rd party cookies uit te zetten in je browser instellingen als je ze niet wilt hebben. Iets wat je toch al aan hebt staan omdat je zo huiverig voor cookies bent, toch?
1.513 accessors? Ga gauw eens Lombok integreren, ik betwijfel of die accessors meer zijn dan

[code]
public String getString() {
return this.string;
}
[/code]

wat natuurlijk keihard boilerplate code is. Of, als jullie avontuurlijk zijn, ga polyglot programmeren en gebruik Scala's case classes voor 'data' classes.

En die duplicatie en gebrek aan test coverage kan natuurlijk echt niet, :p.
Dat zijn inderdaad de standaard triviale accessors. Gelukkig kan IDEA of Eclipse die automatisch voor je genereren, waardoor we niet hoeven te klooien met allerlei andere vage talen.
Sommige private vars hoeven bijvoorbeeld helemaal geen accessor te hebben.

We hebben Sonar sowieso nog niet helemaal naar wens en smaak afgestemd en onder andere nog niet geintegreerd met de unittests. De effectieve coverage is wel wat hoger dan hier staat, hoewel zeker niet zo hoog als zou moeten. Afgezien daarvan vermoed ik dat de duplication niet helemaal te vermijden is, maar we hebben sowieso nog niet niet heel uitgebreid gezocht waar we wat aan (zouden) moeten doen.
Gelukkig kan IDEA of Eclipse die automatisch voor je genereren, waardoor we niet hoeven te klooien met allerlei andere vage talen.
Sommige private vars hoeven bijvoorbeeld helemaal geen accessor te hebben.
Lombok is echter geen vage taal, maar een annotation processor. Daarmee verplaats je het genereren van getters en setters van je IDE naar je compiler, waardoor je source overzichtelijker blijft. Best leuk idee, alhoewel ik het zelf ook niet gebruik. :)

IDE's snappen dit over het algemeen ook gewoon, omdat support voor annotation processors een van de standaardfeatures van Java 6 is.
Ik ben een maand of 2 geleden bij het beginnen aan een nieuwe baan voor het eerst in aanraking gekomen met Lombok. Heerlijk! Het maakt je classes zoveel compacter! Het scheelt niet zozeer in het werk (annotatie typen of toetscombi indrukken in IntelliJ om boiletplate te genereren, is verwaarloosbaar) maar vooral in overzichelijkheid en leesbaarheid. Bovendien bij echt pure data/model classes scheelt het wel echt in werk (kun je nl gewoon de gehele class annoteren).
In de karmaberekening hebben we een aanpassing gedaan om de schaalverandering van de reviews te verwerken. Sinds Tweakers 7 worden reviews niet meer met 1 tot 5 gewaardeerd, maar met een score van 0 tot 3. Nu krijg je weer de juiste karma voor een review die als +3 is beoordeeld.
Hmm, dat maakt nog een best verschil volgens mij. Alles bij elkaar heb ik nu 20K erbij sinds begin van dit jaar :P
Ik vermoed dat je voorlopig nog wel onze karma-koning blijft. ;)
Krijgen we ooit weer de oude wel/niet kruisjes/vinkjes terug in de pricewatch?

Mateloos irritant dat ik kan aanklikken wat ik WEL wil maar niet wat ik NIET wil. Waarom functionalitiet eruit slopen?

Nu kom ik bij cases uit op Micro-ATX : WEL, alleen krijg ik dan ook allemaal ATX kasten. Als ik daar nou net zo als vroeger een kruisje bij kon zetten was het probleem opgelost en zou ik alleen maar kleinere kasten krijgen.
Ja, die zullen wel een keer terugkeren. Helaas zijn er nog heel veel dingen die we ook willen doen en worden we gehinderd door eindige hoeveelheid tijd. Dus ik kan je niet vertellen wanneer dat wordt :)
Dat maakt niet zoveel uit, als jullie maar weten dat er ooit iets aan gedaan moet worden ben ik al blij.

Op dit item kan niet meer gereageerd worden.